[dpdk-dev,PATCHv8,19/46] pool/dpaa2: add DPAA2 hardware offloaded mempool

Message ID 1488545223-25739-20-git-send-email-hemant.agrawal@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Hemant Agrawal March 3, 2017, 12:46 p.m. UTC
  Adding NXP DPAA2 architecture specific mempool support.

This patch also registers a dpaa2 type MEMPOOL OPS

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 MAINTAINERS                                   |   1 +
 config/common_base                            |   5 +
 config/defconfig_arm64-dpaa2-linuxapp-gcc     |   8 +
 drivers/Makefile                              |   1 +
 drivers/pool/Makefile                         |  40 +++
 drivers/pool/dpaa2/Makefile                   |  72 ++++++
 drivers/pool/dpaa2/dpaa2_hw_mempool.c         | 339 ++++++++++++++++++++++++++
 drivers/pool/dpaa2/dpaa2_hw_mempool.h         |  95 ++++++++
 drivers/pool/dpaa2/rte_pool_dpaa2_version.map |   8 +
 mk/rte.app.mk                                 |   1 +
 10 files changed, 570 insertions(+)
 create mode 100644 drivers/pool/Makefile
 create mode 100644 drivers/pool/dpaa2/Makefile
 create mode 100644 drivers/pool/dpaa2/dpaa2_hw_mempool.c
 create mode 100644 drivers/pool/dpaa2/dpaa2_hw_mempool.h
 create mode 100644 drivers/pool/dpaa2/rte_pool_dpaa2_version.map
  

Comments

Ferruh Yigit March 7, 2017, 4:24 p.m. UTC | #1
On 3/3/2017 12:46 PM, Hemant Agrawal wrote:
> Adding NXP DPAA2 architecture specific mempool support.
> 
> This patch also registers a dpaa2 type MEMPOOL OPS
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>

Cc'ing Olivier and Dave.

Hi Olivier, Dave,

Can you please help reviewing this patch?

Thanks,
ferruh

<...>
  
Olivier Matz March 8, 2017, 9:05 a.m. UTC | #2
Hi Hemant,

On Fri, 3 Mar 2017 18:16:36 +0530, Hemant Agrawal
<hemant.agrawal@nxp.com> wrote:
> Adding NXP DPAA2 architecture specific mempool support.
> 
> This patch also registers a dpaa2 type MEMPOOL OPS
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
>  MAINTAINERS                                   |   1 +
>  config/common_base                            |   5 +
>  config/defconfig_arm64-dpaa2-linuxapp-gcc     |   8 +
>  drivers/Makefile                              |   1 +
>  drivers/pool/Makefile                         |  40 +++
>  drivers/pool/dpaa2/Makefile                   |  72 ++++++
>  drivers/pool/dpaa2/dpaa2_hw_mempool.c         | 339
> ++++++++++++++++++++++++++
> drivers/pool/dpaa2/dpaa2_hw_mempool.h         |  95 ++++++++
> drivers/pool/dpaa2/rte_pool_dpaa2_version.map |   8 +

I think the current mempool handlers should be moved first in a
separate patch.

I'd prefer drivers/mempool instead of drivers/pool (more precise and
more consistent with librte_mempool).


>
> [...]
>
> +
> +struct dpaa2_bp_info rte_dpaa2_bpid_info[MAX_BPID];
> +static struct dpaa2_bp_list *h_bp_list;
> +
> +static int
> +hw_mbuf_create_pool(struct rte_mempool *mp)

Would it work for something else than mbufs?
The initial approach of the mempool is to work for kind of object. The
specialization in mbuf is done by the mbuf layer.


> +{
> +	struct dpaa2_bp_list *bp_list;
> +	struct dpaa2_dpbp_dev *avail_dpbp;
> +	struct dpbp_attr dpbp_attr;
> +	uint32_t bpid;
> +	int ret;
> +
> +	avail_dpbp = dpaa2_alloc_dpbp_dev();
> +
> +	if (!avail_dpbp) {
> +		PMD_DRV_LOG(ERR, "DPAA2 resources not available");
> +		return -1;
> +	}

The other pool handlers return a -errno instead of -1. I think it
should be the same here.

The same comment can applies to other locations/functions.

> [...]
> +
> +	/* Set parameters of buffer pool list */
> +	bp_list->buf_pool.num_bufs = mp->size;
> +	bp_list->buf_pool.size = mp->elt_size
> +			- sizeof(struct rte_mbuf) - rte_pktmbuf_priv_size(mp);
> +	bp_list->buf_pool.bpid = dpbp_attr.bpid;
> +	bp_list->buf_pool.h_bpool_mem = NULL;
> +	bp_list->buf_pool.mp = mp;
> +	bp_list->buf_pool.dpbp_node = avail_dpbp;
> +	bp_list->next = h_bp_list;
> +
> +	bpid = dpbp_attr.bpid;
> +
> +
> +	rte_dpaa2_bpid_info[bpid].meta_data_size = sizeof(struct rte_mbuf)
> +				+ rte_pktmbuf_priv_size(mp);

Are the 2 empty lines garbage?


> +	rte_dpaa2_bpid_info[bpid].bp_list = bp_list;
> +	rte_dpaa2_bpid_info[bpid].bpid = bpid;
> +
> +	mp->pool_data = (void *)&rte_dpaa2_bpid_info[bpid];
> +
> +	PMD_INIT_LOG(DEBUG, "BP List created for bpid =%d", dpbp_attr.bpid); +
> +	h_bp_list = bp_list;
> +	/* Identification for our offloaded pool_data structure
> +	 */
> +	mp->flags |= MEMPOOL_F_HW_PKT_POOL;

I think this flag should be declared in rte_mempool.h,
not in drivers/bus/fslmc/portal/dpaa2_hw_pvt.h.

It should also be documented, what does this flag mean?

> [...]
>
> +static
> +void rte_dpaa2_mbuf_release(struct rte_mempool *pool __rte_unused,
> +			void * const *obj_table,
> +			uint32_t bpid,
> +			uint32_t meta_data_size,
> +			int count)


Is there a reason why some functions are prefixed with rte_dpaa2_ and
other but hw_mbuf_?


> +{
> +	struct qbman_release_desc releasedesc;
> +	struct qbman_swp *swp;
> +	int ret;
> +	int i, n;
> +	uint64_t bufs[DPAA2_MBUF_MAX_ACQ_REL];
> +
> +	if (unlikely(!DPAA2_PER_LCORE_DPIO)) {
> +		ret = dpaa2_affine_qbman_swp();
> +		if (ret != 0) {
> +			RTE_LOG(ERR, PMD, "Failed to allocate IO portal");
> +			return;
> +		}
> +	}
> +	swp = DPAA2_PER_LCORE_PORTAL;
> +
> +	/* Create a release descriptor required for releasing
> +	 * buffers into QBMAN
> +	 */
> +	qbman_release_desc_clear(&releasedesc);
> +	qbman_release_desc_set_bpid(&releasedesc, bpid);
> +
> +	n = count % DPAA2_MBUF_MAX_ACQ_REL;
> +
> +	/* convert mbuf to buffers  for the remainder*/

bad spaces

> +	for (i = 0; i < n ; i++)
> +		bufs[i] = (uint64_t)obj_table[i] + meta_data_size;
> +
> +	/* feed them to bman*/

missing space at the end

> +	do {
> +		ret = qbman_swp_release(swp, &releasedesc, bufs, n);
> +	} while (ret == -EBUSY);
> +
> +	/* if there are more buffers to free */
> +	while (n < count) {
> +		/* convert mbuf to buffers */
> +		for (i = 0; i < DPAA2_MBUF_MAX_ACQ_REL; i++)
> +			bufs[i] = (uint64_t)obj_table[n + i] + meta_data_size;
> +
> +		do {
> +			ret = qbman_swp_release(swp, &releasedesc, bufs,
> +						DPAA2_MBUF_MAX_ACQ_REL);
> +			} while (ret == -EBUSY);

The while in not properly indented

> [...]
> +int rte_dpaa2_mbuf_alloc_bulk(struct rte_mempool *pool,
> +		       void **obj_table, unsigned int count)
> +{
> +#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
> +	static int alloc;
> +#endif
> +	struct qbman_swp *swp;
> +	uint32_t mbuf_size;
> +	uint16_t bpid;
> +	uint64_t bufs[DPAA2_MBUF_MAX_ACQ_REL];
> +	int i, ret;
> +	unsigned int n = 0;
> +	struct dpaa2_bp_info *bp_info;
> +
> +	bp_info = mempool_to_bpinfo(pool);
> +
> +	if (!(bp_info->bp_list)) {
> +		RTE_LOG(ERR, PMD, "DPAA2 buffer pool not configured\n");
> +		return -2;
> +	}
> +
> +	bpid = bp_info->bpid;
> +
> +	if (unlikely(!DPAA2_PER_LCORE_DPIO)) {
> +		ret = dpaa2_affine_qbman_swp();
> +		if (ret != 0) {
> +			RTE_LOG(ERR, PMD, "Failed to allocate IO portal");
> +			return -1;
> +		}
> +	}
> +	swp = DPAA2_PER_LCORE_PORTAL;
> +
> +	mbuf_size = sizeof(struct rte_mbuf) + rte_pktmbuf_priv_size(pool);
> +	while (n < count) {
> +		/* Acquire is all-or-nothing, so we drain in 7s,
> +		 * then the remainder.
> +		 */
> +		if ((count - n) > DPAA2_MBUF_MAX_ACQ_REL) {
> +			ret = qbman_swp_acquire(swp, bpid, bufs,
> +					DPAA2_MBUF_MAX_ACQ_REL);
> +		} else {
> +			ret = qbman_swp_acquire(swp, bpid, bufs,
> +						count - n);
> +		}
> +		/* In case of less than requested number of buffers available
> +		 * in pool, qbman_swp_acquire returns 0
> +		 */
> +		if (ret <= 0) {
> +			PMD_TX_LOG(ERR, "Buffer acquire failed with"
> +				   " err code: %d", ret);
> +			/* The API expect the exact number of requested bufs */
> +			/* Releasing all buffers allocated */
> +			rte_dpaa2_mbuf_release(pool, obj_table, bpid,
> +					   bp_info->meta_data_size, n);
> +			return -1;
> +		}
> +		/* assigning mbuf from the acquired objects */
> +		for (i = 0; (i < ret) && bufs[i]; i++) {
> +			/* TODO-errata - observed that bufs may be null
> +			 * i.e. first buffer is valid,
> +			 * remaining 6 buffers may be null
> +			 */
> +			obj_table[n] = (struct rte_mbuf *)(bufs[i] - mbuf_size);
> +			rte_mbuf_refcnt_set((struct rte_mbuf *)obj_table[n], 0);

I think we should not assume the objects are mbufs.
But even if we do it, I don't see why we need to set the refcnt.

What is returned un buf[] table? In rte_dpaa2_mbuf_release(), it looks you
are using buf[i] = obj_table[i] + bp_info->meta_data_size


> [...]
> +
> +static unsigned
> +hw_mbuf_get_count(const struct rte_mempool *mp __rte_unused)
> +{
> +	return 0;

Looks this is not implemented. This would give wrong mempool statistics
when calling rte_mempool_dump().

Adding a test for this handler in app/test may highlight these issues.


> [...]
> +
> +#define DPAA2_MAX_BUF_POOLS	8
> +
> +struct buf_pool_cfg {
> +	void *addr; /*!< The address from where DPAA2 will carve out the
> +		     * buffers. 'addr' should be 'NULL' if user wants
> +		     * to create buffers from the memory which user
> +		     * asked DPAA2 to reserve during 'nadk init'
> +		     */
> +	phys_addr_t    phys_addr;  /*!< corresponding physical address
> +				    * of the memory provided in addr
> +				    */
> +	uint32_t num; /*!< number of buffers */
> +	uint32_t size; /*!< size of each buffer. 'size' should include
> +			* any headroom to be reserved and alignment
> +			*/
> +	uint16_t align; /*!< Buffer alignment (in bytes) */
> +	uint16_t bpid; /*!< The buffer pool id. This will be filled
> +			*in by DPAA2 for each buffer pool
> +			*/
> +};

I think the usual doxygen comment in dpdk is "/**" instead of "/*!"


Regards,
Olivier
  
Hemant Agrawal March 8, 2017, 12:52 p.m. UTC | #3
Hi Olivier,
	Thanks for your detailed review.  Please see inline...

On 3/8/2017 2:35 PM, Olivier MATZ wrote:
> Hi Hemant,
>
> On Fri, 3 Mar 2017 18:16:36 +0530, Hemant Agrawal
> <hemant.agrawal@nxp.com> wrote:
>> Adding NXP DPAA2 architecture specific mempool support.
>>
>> This patch also registers a dpaa2 type MEMPOOL OPS
>>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>>  MAINTAINERS                                   |   1 +
>>  config/common_base                            |   5 +
>>  config/defconfig_arm64-dpaa2-linuxapp-gcc     |   8 +
>>  drivers/Makefile                              |   1 +
>>  drivers/pool/Makefile                         |  40 +++
>>  drivers/pool/dpaa2/Makefile                   |  72 ++++++
>>  drivers/pool/dpaa2/dpaa2_hw_mempool.c         | 339
>> ++++++++++++++++++++++++++
>> drivers/pool/dpaa2/dpaa2_hw_mempool.h         |  95 ++++++++
>> drivers/pool/dpaa2/rte_pool_dpaa2_version.map |   8 +
>
> I think the current mempool handlers should be moved first in a
> separate patch.
>

Are you seeing any benefit by making it a separate patch series?

it will be difficult and tricky for us. The dpaa2_pool has a dependency 
on mc bus patches. dpaa2_pmd has dependency on dpaa2_pool and mc buses.

This will mean that we have to split it into 3 patch series and it will 
become cumbersome to deal with 3 series.


> I'd prefer drivers/mempool instead of drivers/pool (more precise and
> more consistent with librte_mempool).
>

We will take care of it in next revision.

>
>>
>> [...]
>>
>> +
>> +struct dpaa2_bp_info rte_dpaa2_bpid_info[MAX_BPID];
>> +static struct dpaa2_bp_list *h_bp_list;
>> +
>> +static int
>> +hw_mbuf_create_pool(struct rte_mempool *mp)
>
> Would it work for something else than mbufs?
> The initial approach of the mempool is to work for kind of object. The
> specialization in mbuf is done by the mbuf layer.

I think, we did discuss that hw offloaded mempool are mainly for packet 
buffers/mbufs. Currently we only support mbuf type of objects.

Ideally a hw buffer pool can work for any kind mempool. However, it is 
not the best way to use hw buffer pools. The latency to allocate buffers 
are higher than software.  The main advantage SoCs, get by using hw pool 
is that they work seamlessly with the MAC layer.

>
>
>> +{
>> +	struct dpaa2_bp_list *bp_list;
>> +	struct dpaa2_dpbp_dev *avail_dpbp;
>> +	struct dpbp_attr dpbp_attr;
>> +	uint32_t bpid;
>> +	int ret;
>> +
>> +	avail_dpbp = dpaa2_alloc_dpbp_dev();
>> +
>> +	if (!avail_dpbp) {
>> +		PMD_DRV_LOG(ERR, "DPAA2 resources not available");
>> +		return -1;
>> +	}
>
> The other pool handlers return a -errno instead of -1. I think it
> should be the same here.

We will fix it.

>
> The same comment can applies to other locations/functions.
>
>> [...]
>> +
>> +	/* Set parameters of buffer pool list */
>> +	bp_list->buf_pool.num_bufs = mp->size;
>> +	bp_list->buf_pool.size = mp->elt_size
>> +			- sizeof(struct rte_mbuf) - rte_pktmbuf_priv_size(mp);
>> +	bp_list->buf_pool.bpid = dpbp_attr.bpid;
>> +	bp_list->buf_pool.h_bpool_mem = NULL;
>> +	bp_list->buf_pool.mp = mp;
>> +	bp_list->buf_pool.dpbp_node = avail_dpbp;
>> +	bp_list->next = h_bp_list;
>> +
>> +	bpid = dpbp_attr.bpid;
>> +
>> +
>> +	rte_dpaa2_bpid_info[bpid].meta_data_size = sizeof(struct rte_mbuf)
>> +				+ rte_pktmbuf_priv_size(mp);
>
> Are the 2 empty lines garbage?

we will fix it

>
>
>> +	rte_dpaa2_bpid_info[bpid].bp_list = bp_list;
>> +	rte_dpaa2_bpid_info[bpid].bpid = bpid;
>> +
>> +	mp->pool_data = (void *)&rte_dpaa2_bpid_info[bpid];
>> +
>> +	PMD_INIT_LOG(DEBUG, "BP List created for bpid =%d", dpbp_attr.bpid); +
>> +	h_bp_list = bp_list;
>> +	/* Identification for our offloaded pool_data structure
>> +	 */
>> +	mp->flags |= MEMPOOL_F_HW_PKT_POOL;
>
> I think this flag should be declared in rte_mempool.h,
> not in drivers/bus/fslmc/portal/dpaa2_hw_pvt.h.
>
> It should also be documented, what does this flag mean?

Currently we need a way to differentiate that this is a hw allocated pkt 
buffer pool or software based buffer pool. String comparison is costly.

This flag was discussed during the hw mempool patches of david. Not 
everyone was in favor of keeping it in librte_mempool.

So, we just hid it inside our offloaded mempool.

>
>> [...]
>>
>> +static
>> +void rte_dpaa2_mbuf_release(struct rte_mempool *pool __rte_unused,
>> +			void * const *obj_table,
>> +			uint32_t bpid,
>> +			uint32_t meta_data_size,
>> +			int count)
>
>
> Is there a reason why some functions are prefixed with rte_dpaa2_ and
> other but hw_mbuf_?

initial reason was to use rte_ only for exported functions. we can fix it.

>
>
>> +{
>> +	struct qbman_release_desc releasedesc;
>> +	struct qbman_swp *swp;
>> +	int ret;
>> +	int i, n;
>> +	uint64_t bufs[DPAA2_MBUF_MAX_ACQ_REL];
>> +
>> +	if (unlikely(!DPAA2_PER_LCORE_DPIO)) {
>> +		ret = dpaa2_affine_qbman_swp();
>> +		if (ret != 0) {
>> +			RTE_LOG(ERR, PMD, "Failed to allocate IO portal");
>> +			return;
>> +		}
>> +	}
>> +	swp = DPAA2_PER_LCORE_PORTAL;
>> +
>> +	/* Create a release descriptor required for releasing
>> +	 * buffers into QBMAN
>> +	 */
>> +	qbman_release_desc_clear(&releasedesc);
>> +	qbman_release_desc_set_bpid(&releasedesc, bpid);
>> +
>> +	n = count % DPAA2_MBUF_MAX_ACQ_REL;
>> +
>> +	/* convert mbuf to buffers  for the remainder*/
>
> bad spaces

ok

>
>> +	for (i = 0; i < n ; i++)
>> +		bufs[i] = (uint64_t)obj_table[i] + meta_data_size;
>> +
>> +	/* feed them to bman*/
>
> missing space at the end

ok

>
>> +	do {
>> +		ret = qbman_swp_release(swp, &releasedesc, bufs, n);
>> +	} while (ret == -EBUSY);
>> +
>> +	/* if there are more buffers to free */
>> +	while (n < count) {
>> +		/* convert mbuf to buffers */
>> +		for (i = 0; i < DPAA2_MBUF_MAX_ACQ_REL; i++)
>> +			bufs[i] = (uint64_t)obj_table[n + i] + meta_data_size;
>> +
>> +		do {
>> +			ret = qbman_swp_release(swp, &releasedesc, bufs,
>> +						DPAA2_MBUF_MAX_ACQ_REL);
>> +			} while (ret == -EBUSY);
>
> The while in not properly indented

ok

>
>> [...]
>> +int rte_dpaa2_mbuf_alloc_bulk(struct rte_mempool *pool,
>> +		       void **obj_table, unsigned int count)
>> +{
>> +#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
>> +	static int alloc;
>> +#endif
>> +	struct qbman_swp *swp;
>> +	uint32_t mbuf_size;
>> +	uint16_t bpid;
>> +	uint64_t bufs[DPAA2_MBUF_MAX_ACQ_REL];
>> +	int i, ret;
>> +	unsigned int n = 0;
>> +	struct dpaa2_bp_info *bp_info;
>> +
>> +	bp_info = mempool_to_bpinfo(pool);
>> +
>> +	if (!(bp_info->bp_list)) {
>> +		RTE_LOG(ERR, PMD, "DPAA2 buffer pool not configured\n");
>> +		return -2;
>> +	}
>> +
>> +	bpid = bp_info->bpid;
>> +
>> +	if (unlikely(!DPAA2_PER_LCORE_DPIO)) {
>> +		ret = dpaa2_affine_qbman_swp();
>> +		if (ret != 0) {
>> +			RTE_LOG(ERR, PMD, "Failed to allocate IO portal");
>> +			return -1;
>> +		}
>> +	}
>> +	swp = DPAA2_PER_LCORE_PORTAL;
>> +
>> +	mbuf_size = sizeof(struct rte_mbuf) + rte_pktmbuf_priv_size(pool);
>> +	while (n < count) {
>> +		/* Acquire is all-or-nothing, so we drain in 7s,
>> +		 * then the remainder.
>> +		 */
>> +		if ((count - n) > DPAA2_MBUF_MAX_ACQ_REL) {
>> +			ret = qbman_swp_acquire(swp, bpid, bufs,
>> +					DPAA2_MBUF_MAX_ACQ_REL);
>> +		} else {
>> +			ret = qbman_swp_acquire(swp, bpid, bufs,
>> +						count - n);
>> +		}
>> +		/* In case of less than requested number of buffers available
>> +		 * in pool, qbman_swp_acquire returns 0
>> +		 */
>> +		if (ret <= 0) {
>> +			PMD_TX_LOG(ERR, "Buffer acquire failed with"
>> +				   " err code: %d", ret);
>> +			/* The API expect the exact number of requested bufs */
>> +			/* Releasing all buffers allocated */
>> +			rte_dpaa2_mbuf_release(pool, obj_table, bpid,
>> +					   bp_info->meta_data_size, n);
>> +			return -1;
>> +		}
>> +		/* assigning mbuf from the acquired objects */
>> +		for (i = 0; (i < ret) && bufs[i]; i++) {
>> +			/* TODO-errata - observed that bufs may be null
>> +			 * i.e. first buffer is valid,
>> +			 * remaining 6 buffers may be null
>> +			 */
>> +			obj_table[n] = (struct rte_mbuf *)(bufs[i] - mbuf_size);
>> +			rte_mbuf_refcnt_set((struct rte_mbuf *)obj_table[n], 0);
>
> I think we should not assume the objects are mbufs.

currently we are only supporting packet buffer i.e. mbufs

> But even if we do it, I don't see why we need to set the refcnt.

rte_mbuf_raw_alloc has check.
RTE_ASSERT(rte_mbuf_refcnt_read(m) == 0);

So, mempool should have packets with refcnt as '0'.

In our case, during transmit the NIC releases the buffer back to the hw 
pool without core intervention. We have no option to reset specific 
fields of buffer in the hardware.

It can be set on per packet basis during transmission, but this will 
than add to the packet processing cost. In case of simple forwarding, 
NIC will directly get the packets from hw, it will release directly to 
hw. So, we tried to avoid this cost in data path.

>
> What is returned un buf[] table? In rte_dpaa2_mbuf_release(), it looks you
> are using buf[i] = obj_table[i] + bp_info->meta_data_size
>

meta_data_size = sizeof(struct rte_mbuf) +  rte_pktmbuf_priv_size(mp);

In the hardware, we are configure address as buf, which is obj_table + 
meta_data_size;

>
>> [...]
>> +
>> +static unsigned
>> +hw_mbuf_get_count(const struct rte_mempool *mp __rte_unused)
>> +{
>> +	return 0;
>
> Looks this is not implemented. This would give wrong mempool statistics
> when calling rte_mempool_dump().
>

Now our MC buf supports the counts, so we can implement it in next version.

> Adding a test for this handler in app/test may highlight these issues.

Yes! we know it fails. It was not supported in our bus - objects earlier.

Thanks for the suggestion. We have a plan to add these test cases. We 
will add it in our pending item list.

>
>
>> [...]
>> +
>> +#define DPAA2_MAX_BUF_POOLS	8
>> +
>> +struct buf_pool_cfg {
>> +	void *addr; /*!< The address from where DPAA2 will carve out the
>> +		     * buffers. 'addr' should be 'NULL' if user wants
>> +		     * to create buffers from the memory which user
>> +		     * asked DPAA2 to reserve during 'nadk init'
>> +		     */
>> +	phys_addr_t    phys_addr;  /*!< corresponding physical address
>> +				    * of the memory provided in addr
>> +				    */
>> +	uint32_t num; /*!< number of buffers */
>> +	uint32_t size; /*!< size of each buffer. 'size' should include
>> +			* any headroom to be reserved and alignment
>> +			*/
>> +	uint16_t align; /*!< Buffer alignment (in bytes) */
>> +	uint16_t bpid; /*!< The buffer pool id. This will be filled
>> +			*in by DPAA2 for each buffer pool
>> +			*/
>> +};
>
> I think the usual doxygen comment in dpdk is "/**" instead of "/*!"

yes! we will fix it.

>
>
> Regards,
> Olivier
>
Regards,
Hemant
  
Thomas Monjalon March 8, 2017, 3:39 p.m. UTC | #4
2017-03-08 18:22, Hemant Agrawal:
> > On Fri, 3 Mar 2017 18:16:36 +0530, Hemant Agrawal
> > <hemant.agrawal@nxp.com> wrote:
> > I think the current mempool handlers should be moved first in a
> > separate patch.

Yes it should have been done earlier.

> Are you seeing any benefit by making it a separate patch series?

A separate patchset for moving mempool handlers will be easy to review
and accept.
If integrated in this series, it is kind of hidden and prevent the
visibility and review it deserves.
By the way the mempool move should be directly committed in the main
repository, while this series targets next-net.

> it will be difficult and tricky for us. The dpaa2_pool has a dependency 
> on mc bus patches. dpaa2_pmd has dependency on dpaa2_pool and mc buses.

You will just have to rebase this series on top of the new one.
  
Hemant Agrawal March 9, 2017, 5:57 a.m. UTC | #5
On 3/8/2017 9:09 PM, Thomas Monjalon wrote:
> 2017-03-08 18:22, Hemant Agrawal:
>>> On Fri, 3 Mar 2017 18:16:36 +0530, Hemant Agrawal
>>> <hemant.agrawal@nxp.com> wrote:
>>> I think the current mempool handlers should be moved first in a
>>> separate patch.
>
> Yes it should have been done earlier.
>
>> Are you seeing any benefit by making it a separate patch series?
>
> A separate patchset for moving mempool handlers will be easy to review
> and accept.
> If integrated in this series, it is kind of hidden and prevent the
> visibility and review it deserves.
> By the way the mempool move should be directly committed in the main
> repository, while this series targets next-net.
>

hw mempool has dependency on mc-bus. So, we will break it into *5* series,

1. mc-bus - which can go on main repo - Rebased over main repo
2. dpaa2-pool - it can also go on main rep (depends on mc-bus) - Rebased 
over 'main repo + 1'.
3. dpaa2-pmd - this depends on mc-bus and dpaa2-pool
       A. mc-bus -another series rebased over net-next
       B. dpaa2-pool - another series rebased over 'net-next + A'
       C. the pmd itself rebased over 'net-next + A + B'

Are you sure, you will like us to do the above and flood the mailing 
list :)

I think, this will make the review difficult.

Bruce, Ferruh, Olivier - does the above help you? If so, we can do it.

My preference is to keep it as it is. It can be applied on 'net-next' or 
'main' as a whole. But I am open for suggestions.

>> it will be difficult and tricky for us. The dpaa2_pool has a dependency
>> on mc bus patches. dpaa2_pmd has dependency on dpaa2_pool and mc buses.
>
> You will just have to rebase this series on top of the new one.
>
  
Hemant Agrawal March 14, 2017, 6:42 a.m. UTC | #6
On 3/9/2017 11:27 AM, Hemant Agrawal wrote:
> On 3/8/2017 9:09 PM, Thomas Monjalon wrote:
>> 2017-03-08 18:22, Hemant Agrawal:
>>>> On Fri, 3 Mar 2017 18:16:36 +0530, Hemant Agrawal
>>>> <hemant.agrawal@nxp.com> wrote:
>>>> I think the current mempool handlers should be moved first in a
>>>> separate patch.
>>
>> Yes it should have been done earlier.
>>
>>> Are you seeing any benefit by making it a separate patch series?
>>
>> A separate patchset for moving mempool handlers will be easy to review
>> and accept.
>> If integrated in this series, it is kind of hidden and prevent the
>> visibility and review it deserves.
>> By the way the mempool move should be directly committed in the main
>> repository, while this series targets next-net.
>>
>
> hw mempool has dependency on mc-bus. So, we will break it into *5* series,
>
> 1. mc-bus - which can go on main repo - Rebased over main repo
> 2. dpaa2-pool - it can also go on main rep (depends on mc-bus) - Rebased
> over 'main repo + 1'.
> 3. dpaa2-pmd - this depends on mc-bus and dpaa2-pool
>       A. mc-bus -another series rebased over net-next
>       B. dpaa2-pool - another series rebased over 'net-next + A'
>       C. the pmd itself rebased over 'net-next + A + B'
>
> Are you sure, you will like us to do the above and flood the mailing
> list :)
>
> I think, this will make the review difficult.
>
> Bruce, Ferruh, Olivier - does the above help you? If so, we can do it.

Any views, we can rebase our next version accordingly.
we are looking forward to get it merged in 17.05.

>
> My preference is to keep it as it is. It can be applied on 'net-next' or
> 'main' as a whole. But I am open for suggestions.
>


>>> it will be difficult and tricky for us. The dpaa2_pool has a dependency
>>> on mc bus patches. dpaa2_pmd has dependency on dpaa2_pool and mc buses.
>>
>> You will just have to rebase this series on top of the new one.
>>
>
>
>
  
Olivier Matz March 14, 2017, 8:14 a.m. UTC | #7
Hi Hemant,

On Tue, 14 Mar 2017 12:12:17 +0530, Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
> On 3/9/2017 11:27 AM, Hemant Agrawal wrote:
> > On 3/8/2017 9:09 PM, Thomas Monjalon wrote:  
> >> 2017-03-08 18:22, Hemant Agrawal:  
> >>>> On Fri, 3 Mar 2017 18:16:36 +0530, Hemant Agrawal
> >>>> <hemant.agrawal@nxp.com> wrote:
> >>>> I think the current mempool handlers should be moved first in a
> >>>> separate patch.  
> >>
> >> Yes it should have been done earlier.
> >>  
> >>> Are you seeing any benefit by making it a separate patch series?  
> >>
> >> A separate patchset for moving mempool handlers will be easy to review
> >> and accept.
> >> If integrated in this series, it is kind of hidden and prevent the
> >> visibility and review it deserves.
> >> By the way the mempool move should be directly committed in the main
> >> repository, while this series targets next-net.
> >>  
> >
> > hw mempool has dependency on mc-bus. So, we will break it into *5* series,
> >
> > 1. mc-bus - which can go on main repo - Rebased over main repo
> > 2. dpaa2-pool - it can also go on main rep (depends on mc-bus) - Rebased
> > over 'main repo + 1'.
> > 3. dpaa2-pmd - this depends on mc-bus and dpaa2-pool
> >       A. mc-bus -another series rebased over net-next
> >       B. dpaa2-pool - another series rebased over 'net-next + A'
> >       C. the pmd itself rebased over 'net-next + A + B'
> >
> > Are you sure, you will like us to do the above and flood the mailing
> > list :)
> >
> > I think, this will make the review difficult.
> >
> > Bruce, Ferruh, Olivier - does the above help you? If so, we can do it.  
> 
> Any views, we can rebase our next version accordingly.
> we are looking forward to get it merged in 17.05.

Yes, moving the other mempool drivers in the drivers/ directory
before adding another one there makes sense to me, in order to
avoid having two places for mempool handlers.

As Thomas stated, it's probably better to have it in another
patchset, so it can be quickly reviewed and integrated.

Regards,
Olivier
  

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 3fdcf77..ca86dd7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -361,6 +361,7 @@  M: Hemant Agrawal <hemant.agrawal@nxp.com>
 M: Shreyansh Jain <shreyansh.jain@nxp.com>
 F: drivers/bus/fslmc/
 F: drivers/net/dpaa2/
+F: drivers/pool/dpaa2/
 F: doc/guides/nics/dpaa2.rst
 
 QLogic bnx2x
diff --git a/config/common_base b/config/common_base
index e8299d3..c07a95e 100644
--- a/config/common_base
+++ b/config/common_base
@@ -287,6 +287,11 @@  CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_DRIVER=n
 CONFIG_RTE_LIBRTE_THUNDERX_NICVF_DEBUG_MBOX=n
 
 #
+# Compile Support Libraries for NXP DPAA2
+#
+CONFIG_RTE_LIBRTE_DPAA2_POOL=n
+
+#
 # Compile NXP DPAA2 FSL-MC Bus
 #
 CONFIG_RTE_LIBRTE_FSLMC_BUS=n
diff --git a/config/defconfig_arm64-dpaa2-linuxapp-gcc b/config/defconfig_arm64-dpaa2-linuxapp-gcc
index eb12511..3cdb31b 100644
--- a/config/defconfig_arm64-dpaa2-linuxapp-gcc
+++ b/config/defconfig_arm64-dpaa2-linuxapp-gcc
@@ -42,6 +42,14 @@  CONFIG_RTE_ARCH_ARM_TUNE="cortex-a57+fp+simd"
 CONFIG_RTE_MAX_LCORE=8
 CONFIG_RTE_MAX_NUMA_NODES=1
 
+CONFIG_RTE_PKTMBUF_HEADROOM=256
+
+#
+# Compile Support Libraries for DPAA2
+#
+CONFIG_RTE_LIBRTE_DPAA2_POOL=n
+CONFIG_RTE_MBUF_DEFAULT_MEMPOOL_OPS="dpaa2"
+
 #
 # Compile NXP DPAA2 FSL-MC Bus
 #
diff --git a/drivers/Makefile b/drivers/Makefile
index e937449..b122c14 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -32,6 +32,7 @@ 
 include $(RTE_SDK)/mk/rte.vars.mk
 
 DIRS-y += bus
+DIRS-y += pool
 DIRS-y += net
 DIRS-$(CONFIG_RTE_LIBRTE_CRYPTODEV) += crypto
 
diff --git a/drivers/pool/Makefile b/drivers/pool/Makefile
new file mode 100644
index 0000000..3efc336
--- /dev/null
+++ b/drivers/pool/Makefile
@@ -0,0 +1,40 @@ 
+#   BSD LICENSE
+#
+#   Copyright(c) 2016 NXP. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of NXP nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_PMD),y)
+CONFIG_RTE_LIBRTE_DPAA2_POOL = $(CONFIG_RTE_LIBRTE_DPAA2_PMD)
+endif
+
+DIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += dpaa2
+
+include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/drivers/pool/dpaa2/Makefile b/drivers/pool/dpaa2/Makefile
new file mode 100644
index 0000000..c1b15fd
--- /dev/null
+++ b/drivers/pool/dpaa2/Makefile
@@ -0,0 +1,72 @@ 
+#   BSD LICENSE
+#
+#   Copyright(c) 2016 NXP. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of NXP nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+#
+# library name
+#
+LIB = librte_pool_dpaa2.a
+
+ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_PMD),y)
+CONFIG_RTE_LIBRTE_DPAA2_POOL = $(CONFIG_RTE_LIBRTE_DPAA2_PMD)
+endif
+
+ifeq ($(CONFIG_RTE_LIBRTE_DPAA2_DEBUG_INIT),y)
+CFLAGS += -O0 -g
+CFLAGS += "-Wno-error"
+else
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS)
+endif
+
+CFLAGS += -I$(RTE_SDK)/drivers/bus/fslmc
+CFLAGS += -I$(RTE_SDK)/drivers/bus/fslmc/qbman/include
+CFLAGS += -I$(RTE_SDK)/lib/librte_eal/linuxapp/eal
+
+# versioning export map
+EXPORT_MAP := rte_pool_dpaa2_version.map
+
+# Lbrary version
+LIBABIVER := 1
+
+# all source are stored in SRCS-y
+#
+SRCS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += dpaa2_hw_mempool.c
+
+# library dependencies
+DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_eal
+DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += lib/librte_mempool
+DEPDIRS-$(CONFIG_RTE_LIBRTE_DPAA2_POOL) += drivers/bus/fslmc
+
+LDLIBS += -lrte_bus_fslmc
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/drivers/pool/dpaa2/dpaa2_hw_mempool.c b/drivers/pool/dpaa2/dpaa2_hw_mempool.c
new file mode 100644
index 0000000..0c8de51
--- /dev/null
+++ b/drivers/pool/dpaa2/dpaa2_hw_mempool.c
@@ -0,0 +1,339 @@ 
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright (c) 2016 Freescale Semiconductor, Inc. All rights reserved.
+ *   Copyright (c) 2016 NXP. All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Freescale Semiconductor, Inc nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <string.h>
+#include <stdlib.h>
+#include <fcntl.h>
+#include <errno.h>
+
+#include <rte_mbuf.h>
+#include <rte_ethdev.h>
+#include <rte_malloc.h>
+#include <rte_memcpy.h>
+#include <rte_string_fns.h>
+#include <rte_cycles.h>
+#include <rte_kvargs.h>
+#include <rte_dev.h>
+#include <rte_ethdev.h>
+
+#include <fslmc_logs.h>
+#include <mc/fsl_dpbp.h>
+#include <portal/dpaa2_hw_pvt.h>
+#include <portal/dpaa2_hw_dpio.h>
+#include "dpaa2_hw_mempool.h"
+
+struct dpaa2_bp_info rte_dpaa2_bpid_info[MAX_BPID];
+static struct dpaa2_bp_list *h_bp_list;
+
+static int
+hw_mbuf_create_pool(struct rte_mempool *mp)
+{
+	struct dpaa2_bp_list *bp_list;
+	struct dpaa2_dpbp_dev *avail_dpbp;
+	struct dpbp_attr dpbp_attr;
+	uint32_t bpid;
+	int ret;
+
+	avail_dpbp = dpaa2_alloc_dpbp_dev();
+
+	if (!avail_dpbp) {
+		PMD_DRV_LOG(ERR, "DPAA2 resources not available");
+		return -1;
+	}
+
+	if (unlikely(!DPAA2_PER_LCORE_DPIO)) {
+		ret = dpaa2_affine_qbman_swp();
+		if (ret) {
+			RTE_LOG(ERR, PMD, "Failure in affining portal\n");
+			return 0;
+		}
+	}
+
+	ret = dpbp_enable(&avail_dpbp->dpbp, CMD_PRI_LOW, avail_dpbp->token);
+	if (ret != 0) {
+		PMD_INIT_LOG(ERR, "Resource enable failure with"
+			" err code: %d\n", ret);
+		return -1;
+	}
+
+	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);
+		ret = dpbp_disable(&avail_dpbp->dpbp, CMD_PRI_LOW,
+				   avail_dpbp->token);
+		return -1;
+	}
+
+	/* 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 -1;
+	}
+
+	/* Set parameters of buffer pool list */
+	bp_list->buf_pool.num_bufs = mp->size;
+	bp_list->buf_pool.size = mp->elt_size
+			- sizeof(struct rte_mbuf) - rte_pktmbuf_priv_size(mp);
+	bp_list->buf_pool.bpid = dpbp_attr.bpid;
+	bp_list->buf_pool.h_bpool_mem = NULL;
+	bp_list->buf_pool.mp = mp;
+	bp_list->buf_pool.dpbp_node = avail_dpbp;
+	bp_list->next = h_bp_list;
+
+	bpid = dpbp_attr.bpid;
+
+
+	rte_dpaa2_bpid_info[bpid].meta_data_size = sizeof(struct rte_mbuf)
+				+ rte_pktmbuf_priv_size(mp);
+	rte_dpaa2_bpid_info[bpid].bp_list = bp_list;
+	rte_dpaa2_bpid_info[bpid].bpid = bpid;
+
+	mp->pool_data = (void *)&rte_dpaa2_bpid_info[bpid];
+
+	PMD_INIT_LOG(DEBUG, "BP List created for bpid =%d", dpbp_attr.bpid);
+
+	h_bp_list = bp_list;
+	/* Identification for our offloaded pool_data structure
+	 */
+	mp->flags |= MEMPOOL_F_HW_PKT_POOL;
+	return 0;
+}
+
+static void
+hw_mbuf_free_pool(struct rte_mempool *mp)
+{
+	struct dpaa2_bp_info *bpinfo;
+	struct dpaa2_bp_list *bp;
+	struct dpaa2_dpbp_dev *dpbp_node;
+
+	if (!mp->pool_data) {
+		PMD_DRV_LOG(ERR, "Not a valid dpaa22 pool");
+		return;
+	}
+
+	bpinfo = (struct dpaa2_bp_info *)mp->pool_data;
+	bp = bpinfo->bp_list;
+	dpbp_node = bp->buf_pool.dpbp_node;
+
+	dpbp_disable(&(dpbp_node->dpbp), CMD_PRI_LOW, dpbp_node->token);
+
+	if (h_bp_list == bp) {
+		h_bp_list = h_bp_list->next;
+	} else { /* if it is not the first node */
+		struct dpaa2_bp_list *prev = h_bp_list, *temp;
+		temp = h_bp_list->next;
+		while (temp) {
+			if (temp == bp) {
+				prev->next = temp->next;
+				free(bp);
+				break;
+			}
+			prev = temp;
+			temp = temp->next;
+		}
+	}
+
+	dpaa2_free_dpbp_dev(dpbp_node);
+}
+
+static
+void rte_dpaa2_mbuf_release(struct rte_mempool *pool __rte_unused,
+			void * const *obj_table,
+			uint32_t bpid,
+			uint32_t meta_data_size,
+			int count)
+{
+	struct qbman_release_desc releasedesc;
+	struct qbman_swp *swp;
+	int ret;
+	int i, n;
+	uint64_t bufs[DPAA2_MBUF_MAX_ACQ_REL];
+
+	if (unlikely(!DPAA2_PER_LCORE_DPIO)) {
+		ret = dpaa2_affine_qbman_swp();
+		if (ret != 0) {
+			RTE_LOG(ERR, PMD, "Failed to allocate IO portal");
+			return;
+		}
+	}
+	swp = DPAA2_PER_LCORE_PORTAL;
+
+	/* Create a release descriptor required for releasing
+	 * buffers into QBMAN
+	 */
+	qbman_release_desc_clear(&releasedesc);
+	qbman_release_desc_set_bpid(&releasedesc, bpid);
+
+	n = count % DPAA2_MBUF_MAX_ACQ_REL;
+
+	/* convert mbuf to buffers  for the remainder*/
+	for (i = 0; i < n ; i++)
+		bufs[i] = (uint64_t)obj_table[i] + meta_data_size;
+
+	/* feed them to bman*/
+	do {
+		ret = qbman_swp_release(swp, &releasedesc, bufs, n);
+	} while (ret == -EBUSY);
+
+	/* if there are more buffers to free */
+	while (n < count) {
+		/* convert mbuf to buffers */
+		for (i = 0; i < DPAA2_MBUF_MAX_ACQ_REL; i++)
+			bufs[i] = (uint64_t)obj_table[n + i] + meta_data_size;
+
+		do {
+			ret = qbman_swp_release(swp, &releasedesc, bufs,
+						DPAA2_MBUF_MAX_ACQ_REL);
+			} while (ret == -EBUSY);
+		n += DPAA2_MBUF_MAX_ACQ_REL;
+	}
+}
+
+int rte_dpaa2_mbuf_alloc_bulk(struct rte_mempool *pool,
+		       void **obj_table, unsigned int count)
+{
+#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
+	static int alloc;
+#endif
+	struct qbman_swp *swp;
+	uint32_t mbuf_size;
+	uint16_t bpid;
+	uint64_t bufs[DPAA2_MBUF_MAX_ACQ_REL];
+	int i, ret;
+	unsigned int n = 0;
+	struct dpaa2_bp_info *bp_info;
+
+	bp_info = mempool_to_bpinfo(pool);
+
+	if (!(bp_info->bp_list)) {
+		RTE_LOG(ERR, PMD, "DPAA2 buffer pool not configured\n");
+		return -2;
+	}
+
+	bpid = bp_info->bpid;
+
+	if (unlikely(!DPAA2_PER_LCORE_DPIO)) {
+		ret = dpaa2_affine_qbman_swp();
+		if (ret != 0) {
+			RTE_LOG(ERR, PMD, "Failed to allocate IO portal");
+			return -1;
+		}
+	}
+	swp = DPAA2_PER_LCORE_PORTAL;
+
+	mbuf_size = sizeof(struct rte_mbuf) + rte_pktmbuf_priv_size(pool);
+
+	while (n < count) {
+		/* Acquire is all-or-nothing, so we drain in 7s,
+		 * then the remainder.
+		 */
+		if ((count - n) > DPAA2_MBUF_MAX_ACQ_REL) {
+			ret = qbman_swp_acquire(swp, bpid, bufs,
+						DPAA2_MBUF_MAX_ACQ_REL);
+		} else {
+			ret = qbman_swp_acquire(swp, bpid, bufs,
+						count - n);
+		}
+		/* In case of less than requested number of buffers available
+		 * in pool, qbman_swp_acquire returns 0
+		 */
+		if (ret <= 0) {
+			PMD_TX_LOG(ERR, "Buffer acquire failed with"
+				   " err code: %d", ret);
+			/* The API expect the exact number of requested bufs */
+			/* Releasing all buffers allocated */
+			rte_dpaa2_mbuf_release(pool, obj_table, bpid,
+					   bp_info->meta_data_size, n);
+			return -1;
+		}
+		/* assigning mbuf from the acquired objects */
+		for (i = 0; (i < ret) && bufs[i]; i++) {
+			/* TODO-errata - observed that bufs may be null
+			 * i.e. first buffer is valid,
+			 * remaining 6 buffers may be null
+			 */
+			obj_table[n] = (struct rte_mbuf *)(bufs[i] - mbuf_size);
+			rte_mbuf_refcnt_set((struct rte_mbuf *)obj_table[n], 0);
+			PMD_TX_LOG(DEBUG, "Acquired %p address %p from BMAN",
+				   (void *)bufs[i], (void *)obj_table[n]);
+			n++;
+		}
+	}
+
+#ifdef RTE_LIBRTE_DPAA2_DEBUG_DRIVER
+	alloc += n;
+	PMD_TX_LOG(DEBUG, "Total = %d , req = %d done = %d",
+		   alloc, count, n);
+#endif
+	return 0;
+}
+
+static int
+hw_mbuf_free_bulk(struct rte_mempool *pool,
+		  void * const *obj_table, unsigned int n)
+{
+	struct dpaa2_bp_info *bp_info;
+
+	bp_info = mempool_to_bpinfo(pool);
+	if (!(bp_info->bp_list)) {
+		RTE_LOG(ERR, PMD, "DPAA2 buffer pool not configured");
+		return -1;
+	}
+	rte_dpaa2_mbuf_release(pool, obj_table, bp_info->bpid,
+			   bp_info->meta_data_size, n);
+
+	return 0;
+}
+
+static unsigned
+hw_mbuf_get_count(const struct rte_mempool *mp __rte_unused)
+{
+	return 0;
+}
+
+struct rte_mempool_ops dpaa2_mpool_ops = {
+	.name = "dpaa2",
+	.alloc = hw_mbuf_create_pool,
+	.free = hw_mbuf_free_pool,
+	.enqueue = hw_mbuf_free_bulk,
+	.dequeue = rte_dpaa2_mbuf_alloc_bulk,
+	.get_count = hw_mbuf_get_count,
+};
+
+MEMPOOL_REGISTER_OPS(dpaa2_mpool_ops);
diff --git a/drivers/pool/dpaa2/dpaa2_hw_mempool.h b/drivers/pool/dpaa2/dpaa2_hw_mempool.h
new file mode 100644
index 0000000..c0e2411
--- /dev/null
+++ b/drivers/pool/dpaa2/dpaa2_hw_mempool.h
@@ -0,0 +1,95 @@ 
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright (c) 2016 Freescale Semiconductor, Inc. All rights reserved.
+ *   Copyright (c) 2016 NXP. All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Freescale Semiconductor, Inc nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _DPAA2_HW_DPBP_H_
+#define _DPAA2_HW_DPBP_H_
+
+#define DPAA2_MAX_BUF_POOLS	8
+
+struct buf_pool_cfg {
+	void *addr; /*!< The address from where DPAA2 will carve out the
+		     * buffers. 'addr' should be 'NULL' if user wants
+		     * to create buffers from the memory which user
+		     * asked DPAA2 to reserve during 'nadk init'
+		     */
+	phys_addr_t    phys_addr;  /*!< corresponding physical address
+				    * of the memory provided in addr
+				    */
+	uint32_t num; /*!< number of buffers */
+	uint32_t size; /*!< size of each buffer. 'size' should include
+			* any headroom to be reserved and alignment
+			*/
+	uint16_t align; /*!< Buffer alignment (in bytes) */
+	uint16_t bpid; /*!< The buffer pool id. This will be filled
+			*in by DPAA2 for each buffer pool
+			*/
+};
+
+struct buf_pool {
+	uint32_t size;
+	uint32_t num_bufs;
+	uint16_t bpid;
+	uint8_t *h_bpool_mem;
+	struct rte_mempool *mp;
+	struct dpaa2_dpbp_dev *dpbp_node;
+};
+
+/*!
+ * Buffer pool list configuration structure. User need to give DPAA2 the
+ * valid number of 'num_buf_pools'.
+ */
+struct dpaa2_bp_list_cfg {
+	struct buf_pool_cfg buf_pool; /* Configuration of each buffer pool*/
+};
+
+struct dpaa2_bp_list {
+	struct dpaa2_bp_list *next;
+	struct rte_mempool *mp;
+	struct buf_pool buf_pool;
+};
+
+struct dpaa2_bp_info {
+	uint32_t meta_data_size;
+	uint32_t bpid;
+	struct dpaa2_bp_list *bp_list;
+};
+
+#define mempool_to_bpinfo(mp) ((struct dpaa2_bp_info *)(mp)->pool_data)
+#define mempool_to_bpid(mp) ((mempool_to_bpinfo(mp))->bpid)
+
+extern struct dpaa2_bp_info rte_dpaa2_bpid_info[MAX_BPID];
+
+int rte_dpaa2_mbuf_alloc_bulk(struct rte_mempool *pool,
+		       void **obj_table, unsigned int count);
+
+#endif /* _DPAA2_HW_DPBP_H_ */
diff --git a/drivers/pool/dpaa2/rte_pool_dpaa2_version.map b/drivers/pool/dpaa2/rte_pool_dpaa2_version.map
new file mode 100644
index 0000000..a8aa685
--- /dev/null
+++ b/drivers/pool/dpaa2/rte_pool_dpaa2_version.map
@@ -0,0 +1,8 @@ 
+DPDK_17.05 {
+	global:
+
+	rte_dpaa2_bpid_info;
+	rte_dpaa2_mbuf_alloc_bulk;
+
+	local: *;
+};
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 22c22d4..db5b790 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -109,6 +109,7 @@  _LDLIBS-$(CONFIG_RTE_LIBRTE_BNXT_PMD)       += -lrte_pmd_bnxt
 _LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND)       += -lrte_pmd_bond
 _LDLIBS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD)      += -lrte_pmd_cxgbe
 _LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD)      += -lrte_pmd_dpaa2
+_LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD)      += -lrte_pool_dpaa2
 _LDLIBS-$(CONFIG_RTE_LIBRTE_DPAA2_PMD)      += -lrte_bus_fslmc
 _LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD)      += -lrte_pmd_e1000
 _LDLIBS-$(CONFIG_RTE_LIBRTE_ENA_PMD)        += -lrte_pmd_ena