[PATCH 1/2] net/i40e: replace put function

[PATCH 1/2] net/i40e: replace put function

[Kamalakshitha Aligeri]

Integrated zero-copy put API in mempool cache in i40e PMD.
On Ampere Altra server, l3fwd single core's performance improves by 5%
with the new API

Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
Link: https://patchwork.dpdk.org/project/dpdk/patch/20221227151700.80887-1-mb@smartsharesystems.com/

 .mailmap                                |  1 +
 drivers/net/i40e/i40e_rxtx_vec_common.h | 34 ++++++++++++++++++++-----
 2 files changed, 28 insertions(+), 7 deletions(-)

diff --git a/.mailmap b/.mailmap
index 75884b6fe2..05a42edbcf 100644
--- a/.mailmap
+++ b/.mailmap
@@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>
 Kaiwen Deng <kaiwenx.deng@intel.com>
 Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
 Kamalakannan R <kamalakannan.r@intel.com>
+Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
 Kamil Bednarczyk <kamil.bednarczyk@intel.com>
 Kamil Chalupnik <kamilx.chalupnik@intel.com>
 Kamil Rytarowski <kamil.rytarowski@caviumnetworks.com>
diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h
index fe1a6ec75e..80d4a159e6 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_common.h
+++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
@@ -95,17 +95,37 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)

 	n = txq->tx_rs_thresh;

-	 /* first buffer to free from S/W ring is at index
-	  * tx_next_dd - (tx_rs_thresh-1)
-	  */
+	/* first buffer to free from S/W ring is at index
+	 * tx_next_dd - (tx_rs_thresh-1)
+	 */
 	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];

 	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
-		for (i = 0; i < n; i++) {
-			free[i] = txep[i].mbuf;
-			/* no need to reset txep[i].mbuf in vector path */
+		struct rte_mempool *mp = txep[0].mbuf->pool;
+		struct rte_mempool_cache *cache = rte_mempool_default_cache(mp, rte_lcore_id());
+
+		if (!cache || n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
+			for (i = 0; i < n ; i++)
+				free[i] = txep[i].mbuf;
+			if (!cache) {
+				rte_mempool_generic_put(mp, (void **)free, n, cache);
+				goto done;
+			}
+			if (n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
+				rte_mempool_ops_enqueue_bulk(mp, (void **)free, n);
+				goto done;
+			}
+		}
+		void **cache_objs;
+
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, n);
+		if (cache_objs) {
+			for (i = 0; i < n; i++) {
+				cache_objs[i] = txep->mbuf;
+				/* no need to reset txep[i].mbuf in vector path */
+				txep++;
+			}
 		}
-		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
 		goto done;
 	}

--
2.25.1

[PATCH 2/2] test/mempool: add zero-copy API's

[Kamalakshitha Aligeri]

Added mempool test cases with zero-copy get and put API's

Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
Link: https://patchwork.dpdk.org/project/dpdk/patch/20221227151700.80887-1-mb@smartsharesystems.com/

 app/test/test_mempool.c | 81 ++++++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 21 deletions(-)

diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 8e493eda47..6d29f5bc7b 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -74,7 +74,7 @@ my_obj_init(struct rte_mempool *mp, __rte_unused void *arg,

 /* basic tests (done on one core) */
 static int
-test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
+test_mempool_basic(struct rte_mempool *mp, int use_external_cache, int use_zc_api)
 {
 	uint32_t *objnum;
 	void **objtable;
@@ -84,6 +84,7 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 	unsigned i, j;
 	int offset;
 	struct rte_mempool_cache *cache;
+	void **cache_objs;

 	if (use_external_cache) {
 		/* Create a user-owned mempool cache. */
@@ -100,8 +101,13 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 	rte_mempool_dump(stdout, mp);

 	printf("get an object\n");
-	if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
-		GOTO_ERR(ret, out);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj = *cache_objs;
+	} else {
+		if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
+			GOTO_ERR(ret, out);
+	}
 	rte_mempool_dump(stdout, mp);

 	/* tests that improve coverage */
@@ -123,21 +129,41 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 #endif

 	printf("put the object back\n");
-	rte_mempool_generic_put(mp, &obj, 1, cache);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj, sizeof(void *));
+	} else {
+		rte_mempool_generic_put(mp, &obj, 1, cache);
+	}
 	rte_mempool_dump(stdout, mp);

 	printf("get 2 objects\n");
-	if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
-		GOTO_ERR(ret, out);
-	if (rte_mempool_generic_get(mp, &obj2, 1, cache) < 0) {
-		rte_mempool_generic_put(mp, &obj, 1, cache);
-		GOTO_ERR(ret, out);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj = *cache_objs;
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj2 = *cache_objs;
+	} else {
+		if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
+			GOTO_ERR(ret, out);
+		if (rte_mempool_generic_get(mp, &obj2, 1, cache) < 0) {
+			rte_mempool_generic_put(mp, &obj, 1, cache);
+			GOTO_ERR(ret, out);
+		}
 	}
 	rte_mempool_dump(stdout, mp);

 	printf("put the objects back\n");
-	rte_mempool_generic_put(mp, &obj, 1, cache);
-	rte_mempool_generic_put(mp, &obj2, 1, cache);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj, sizeof(void *));
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj2, sizeof(void *));
+
+	} else {
+		rte_mempool_generic_put(mp, &obj, 1, cache);
+		rte_mempool_generic_put(mp, &obj2, 1, cache);
+	}
 	rte_mempool_dump(stdout, mp);

 	/*
@@ -149,8 +175,13 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 		GOTO_ERR(ret, out);

 	for (i = 0; i < MEMPOOL_SIZE; i++) {
-		if (rte_mempool_generic_get(mp, &objtable[i], 1, cache) < 0)
-			break;
+		if (use_zc_api) {
+			cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+			objtable[i] = *cache_objs;
+		} else {
+			if (rte_mempool_generic_get(mp, &objtable[i], 1, cache) < 0)
+				break;
+		}
 	}

 	/*
@@ -170,8 +201,12 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 			if (obj_data[j] != 0)
 				ret = -1;
 		}
-
-		rte_mempool_generic_put(mp, &objtable[i], 1, cache);
+		if (use_zc_api) {
+			cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+			rte_memcpy(cache_objs, &objtable[i], sizeof(void *));
+		} else {
+			rte_mempool_generic_put(mp, &objtable[i], 1, cache);
+		}
 	}

 	free(objtable);
@@ -979,15 +1014,19 @@ test_mempool(void)
 	rte_mempool_list_dump(stdout);

 	/* basic tests without cache */
-	if (test_mempool_basic(mp_nocache, 0) < 0)
+	if (test_mempool_basic(mp_nocache, 0, 0) < 0)
+		GOTO_ERR(ret, err);
+
+	/* basic tests with zero-copy API's */
+	if (test_mempool_basic(mp_cache, 0, 1) < 0)
 		GOTO_ERR(ret, err);

-	/* basic tests with cache */
-	if (test_mempool_basic(mp_cache, 0) < 0)
+	/* basic tests with user-owned cache and zero-copy API's */
+	if (test_mempool_basic(mp_nocache, 1, 1) < 0)
 		GOTO_ERR(ret, err);

 	/* basic tests with user-owned cache */
-	if (test_mempool_basic(mp_nocache, 1) < 0)
+	if (test_mempool_basic(mp_nocache, 1, 0) < 0)
 		GOTO_ERR(ret, err);

 	/* more basic tests without cache */
@@ -1008,10 +1047,10 @@ test_mempool(void)
 		GOTO_ERR(ret, err);

 	/* test the stack handler */
-	if (test_mempool_basic(mp_stack, 1) < 0)
+	if (test_mempool_basic(mp_stack, 1, 0) < 0)
 		GOTO_ERR(ret, err);

-	if (test_mempool_basic(default_pool, 1) < 0)
+	if (test_mempool_basic(default_pool, 1, 0) < 0)
 		GOTO_ERR(ret, err);

 	/* test mempool event callbacks */
--
2.25.1

RE: [PATCH 2/2] test/mempool: add zero-copy API's

[Morten Brørup]

> From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> Sent: Thursday, 9 February 2023 07.25
> 
> Added mempool test cases with zero-copy get and put API's
> 
> Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> ---
> Link:
> https://patchwork.dpdk.org/project/dpdk/patch/20221227151700.80887-1-
> mb@smartsharesystems.com/
> 

Good choice of test method - testing the same, but also with the zero-copy methods.

Acked-by: Morten Brørup <mb@smartsharesystems.com>

RE: [PATCH 1/2] net/i40e: replace put function

[Morten Brørup]

> From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> Sent: Thursday, 9 February 2023 07.25
> 
> Integrated zero-copy put API in mempool cache in i40e PMD.
> On Ampere Altra server, l3fwd single core's performance improves by 5%
> with the new API
> 
> Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> ---
> Link:
> https://patchwork.dpdk.org/project/dpdk/patch/20221227151700.80887-1-
> mb@smartsharesystems.com/
> 
>  .mailmap                                |  1 +
>  drivers/net/i40e/i40e_rxtx_vec_common.h | 34 ++++++++++++++++++++-----
>  2 files changed, 28 insertions(+), 7 deletions(-)
> 
> diff --git a/.mailmap b/.mailmap
> index 75884b6fe2..05a42edbcf 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>
>  Kaiwen Deng <kaiwenx.deng@intel.com>
>  Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>  Kamalakannan R <kamalakannan.r@intel.com>
> +Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
>  Kamil Bednarczyk <kamil.bednarczyk@intel.com>
>  Kamil Chalupnik <kamilx.chalupnik@intel.com>
>  Kamil Rytarowski <kamil.rytarowski@caviumnetworks.com>
> diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h
> b/drivers/net/i40e/i40e_rxtx_vec_common.h
> index fe1a6ec75e..80d4a159e6 100644
> --- a/drivers/net/i40e/i40e_rxtx_vec_common.h
> +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
> @@ -95,17 +95,37 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)
> 
>  	n = txq->tx_rs_thresh;
> 
> -	 /* first buffer to free from S/W ring is at index
> -	  * tx_next_dd - (tx_rs_thresh-1)
> -	  */
> +	/* first buffer to free from S/W ring is at index
> +	 * tx_next_dd - (tx_rs_thresh-1)
> +	 */
>  	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];
> 
>  	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
> -		for (i = 0; i < n; i++) {
> -			free[i] = txep[i].mbuf;
> -			/* no need to reset txep[i].mbuf in vector path */
> +		struct rte_mempool *mp = txep[0].mbuf->pool;
> +		struct rte_mempool_cache *cache =
> rte_mempool_default_cache(mp, rte_lcore_id());
> +
> +		if (!cache || n > RTE_MEMPOOL_CACHE_MAX_SIZE) {

If the mempool has a cache, do not compare n to RTE_MEMPOOL_CACHE_MAX_SIZE. Instead, call rte_mempool_cache_zc_put_bulk() to determine if n is acceptable for zero-copy.

It looks like this patch behaves incorrectly if the cache is configured to be smaller than RTE_MEMPOOL_CACHE_MAX_SIZE. Let's say the cache size is 8, which will make the flush threshold 12. If n is 32, your code will not enter this branch, but proceed to call rte_mempool_cache_zc_put_bulk(), which will return NULL, and then you will goto done.

Obviously, if there is no cache, fall back to the standard rte_mempool_put_bulk().

> +			for (i = 0; i < n ; i++)
> +				free[i] = txep[i].mbuf;
> +			if (!cache) {
> +				rte_mempool_generic_put(mp, (void **)free, n,
> cache);
> +				goto done;
> +			}
> +			if (n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
> +				rte_mempool_ops_enqueue_bulk(mp, (void **)free,
> n);
> +				goto done;
> +			}
> +		}
> +		void **cache_objs;
> +
> +		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, n);
> +		if (cache_objs) {
> +			for (i = 0; i < n; i++) {
> +				cache_objs[i] = txep->mbuf;
> +				/* no need to reset txep[i].mbuf in vector path
> */
> +				txep++;
> +			}
>  		}
> -		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
>  		goto done;
>  	}
> 
> --
> 2.25.1
> 

回复: [PATCH 1/2] net/i40e: replace put function

[Feifei Wang]

Hi, Morten

> -----邮件原件-----
> 发件人: Morten Brørup <mb@smartsharesystems.com>
> 发送时间: Thursday, February 9, 2023 5:34 PM
> 收件人: Kamalakshitha Aligeri <Kamalakshitha.Aligeri@arm.com>;
> Yuying.Zhang@intel.com; beilei.xing@intel.com; olivier.matz@6wind.com;
> andrew.rybchenko@oktetlabs.ru; bruce.richardson@intel.com;
> konstantin.ananyev@huawei.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> 抄送: dev@dpdk.org; nd <nd@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Feifei Wang <Feifei.Wang2@arm.com>
> 主题: RE: [PATCH 1/2] net/i40e: replace put function
> 
> > From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> > Sent: Thursday, 9 February 2023 07.25
> >
> > Integrated zero-copy put API in mempool cache in i40e PMD.
> > On Ampere Altra server, l3fwd single core's performance improves by 5%
> > with the new API
> >
> > Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> > ---
> > Link:
> > https://patchwork.dpdk.org/project/dpdk/patch/20221227151700.80887-1-
> > mb@smartsharesystems.com/
> >
> >  .mailmap                                |  1 +
> >  drivers/net/i40e/i40e_rxtx_vec_common.h | 34
> > ++++++++++++++++++++-----
> >  2 files changed, 28 insertions(+), 7 deletions(-)
> >
> > diff --git a/.mailmap b/.mailmap
> > index 75884b6fe2..05a42edbcf 100644
> > --- a/.mailmap
> > +++ b/.mailmap
> > @@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>  Kaiwen Deng
> > <kaiwenx.deng@intel.com>  Kalesh AP
> > <kalesh-anakkur.purayil@broadcom.com>
> >  Kamalakannan R <kamalakannan.r@intel.com>
> > +Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> >  Kamil Bednarczyk <kamil.bednarczyk@intel.com>  Kamil Chalupnik
> > <kamilx.chalupnik@intel.com>  Kamil Rytarowski
> > <kamil.rytarowski@caviumnetworks.com>
> > diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h
> > b/drivers/net/i40e/i40e_rxtx_vec_common.h
> > index fe1a6ec75e..80d4a159e6 100644
> > --- a/drivers/net/i40e/i40e_rxtx_vec_common.h
> > +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
> > @@ -95,17 +95,37 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)
> >
> >  	n = txq->tx_rs_thresh;
> >
> > -	 /* first buffer to free from S/W ring is at index
> > -	  * tx_next_dd - (tx_rs_thresh-1)
> > -	  */
> > +	/* first buffer to free from S/W ring is at index
> > +	 * tx_next_dd - (tx_rs_thresh-1)
> > +	 */
> >  	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];
> >
> >  	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
> > -		for (i = 0; i < n; i++) {
> > -			free[i] = txep[i].mbuf;
> > -			/* no need to reset txep[i].mbuf in vector path */
> > +		struct rte_mempool *mp = txep[0].mbuf->pool;
> > +		struct rte_mempool_cache *cache =
> > rte_mempool_default_cache(mp, rte_lcore_id());
> > +
> > +		if (!cache || n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
> 
> If the mempool has a cache, do not compare n to
> RTE_MEMPOOL_CACHE_MAX_SIZE. Instead, call
> rte_mempool_cache_zc_put_bulk() to determine if n is acceptable for zero-
> copy.
> 

> It looks like this patch behaves incorrectly if the cache is configured to be
> smaller than RTE_MEMPOOL_CACHE_MAX_SIZE. Let's say the cache size is 8,
> which will make the flush threshold 12. If n is 32, your code will not enter this
> branch, but proceed to call rte_mempool_cache_zc_put_bulk(), which will
> return NULL, and then you will goto done.
> 
> Obviously, if there is no cache, fall back to the standard
> rte_mempool_put_bulk().

Agree with this. I think we ignore the case that (cache -> flushthresh  < n <  RTE_MEMPOOL_CACHE_MAX_SIZE).

Our goal is that if (!cache || n > cache -> flushthresh), we can put the buffers
into mempool directly.  

Thus maybe we can change as:
struct rte_mempool_cache *cache = rte_mempool_default_cache(mp, rte_lcore_id());
if (!cache || n > cache -> flushthresh) {
      for (i = 0; i < n ; i++)
          free[i] = txep[i].mbuf;
      if (!cache) {
                rte_mempool_generic_put;
                goto done;
      } else if {
                rte_mempool_ops_enqueue_bulk;
                goto done;
      }
}

If we can change like this?

> 
> > +			for (i = 0; i < n ; i++)
> > +				free[i] = txep[i].mbuf;
> > +			if (!cache) {
> > +				rte_mempool_generic_put(mp, (void
> **)free, n,
> > cache);
> > +				goto done;
> > +			}
> > +			if (n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
> > +				rte_mempool_ops_enqueue_bulk(mp, (void
> **)free,
> > n);
> > +				goto done;
> > +			}
> > +		}
> > +		void **cache_objs;
> > +
> > +		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp,
> n);
> > +		if (cache_objs) {
> > +			for (i = 0; i < n; i++) {
> > +				cache_objs[i] = txep->mbuf;
> > +				/* no need to reset txep[i].mbuf in vector
> path
> > */
> > +				txep++;
> > +			}
> >  		}
> > -		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
> >  		goto done;
> >  	}
> >
> > --
> > 2.25.1
> >

RE: [PATCH 1/2] net/i40e: replace put function

[Morten Brørup]

> From: Feifei Wang [mailto:Feifei.Wang2@arm.com]
> Sent: Thursday, 9 February 2023 11.59
> 
> Hi, Morten
> 
> > 发件人: Morten Brørup <mb@smartsharesystems.com>
> > 发送时间: Thursday, February 9, 2023 5:34 PM
> >
> > > From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> > > Sent: Thursday, 9 February 2023 07.25
> > >
> > > Integrated zero-copy put API in mempool cache in i40e PMD.
> > > On Ampere Altra server, l3fwd single core's performance improves by
> 5%
> > > with the new API
> > >
> > > Signed-off-by: Kamalakshitha Aligeri
> <kamalakshitha.aligeri@arm.com>
> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> > > ---
> > > Link:
> > > https://patchwork.dpdk.org/project/dpdk/patch/20221227151700.80887-
> 1-
> > > mb@smartsharesystems.com/
> > >
> > >  .mailmap                                |  1 +
> > >  drivers/net/i40e/i40e_rxtx_vec_common.h | 34
> > > ++++++++++++++++++++-----
> > >  2 files changed, 28 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/.mailmap b/.mailmap
> > > index 75884b6fe2..05a42edbcf 100644
> > > --- a/.mailmap
> > > +++ b/.mailmap
> > > @@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>  Kaiwen Deng
> > > <kaiwenx.deng@intel.com>  Kalesh AP
> > > <kalesh-anakkur.purayil@broadcom.com>
> > >  Kamalakannan R <kamalakannan.r@intel.com>
> > > +Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > >  Kamil Bednarczyk <kamil.bednarczyk@intel.com>  Kamil Chalupnik
> > > <kamilx.chalupnik@intel.com>  Kamil Rytarowski
> > > <kamil.rytarowski@caviumnetworks.com>
> > > diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h
> > > b/drivers/net/i40e/i40e_rxtx_vec_common.h
> > > index fe1a6ec75e..80d4a159e6 100644
> > > --- a/drivers/net/i40e/i40e_rxtx_vec_common.h
> > > +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
> > > @@ -95,17 +95,37 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)
> > >
> > >  	n = txq->tx_rs_thresh;
> > >
> > > -	 /* first buffer to free from S/W ring is at index
> > > -	  * tx_next_dd - (tx_rs_thresh-1)
> > > -	  */
> > > +	/* first buffer to free from S/W ring is at index
> > > +	 * tx_next_dd - (tx_rs_thresh-1)
> > > +	 */
> > >  	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];
> > >
> > >  	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
> > > -		for (i = 0; i < n; i++) {
> > > -			free[i] = txep[i].mbuf;
> > > -			/* no need to reset txep[i].mbuf in vector path */
> > > +		struct rte_mempool *mp = txep[0].mbuf->pool;
> > > +		struct rte_mempool_cache *cache =
> > > rte_mempool_default_cache(mp, rte_lcore_id());
> > > +
> > > +		if (!cache || n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
> >
> > If the mempool has a cache, do not compare n to
> > RTE_MEMPOOL_CACHE_MAX_SIZE. Instead, call
> > rte_mempool_cache_zc_put_bulk() to determine if n is acceptable for
> zero-
> > copy.
> >
> 
> > It looks like this patch behaves incorrectly if the cache is
> configured to be
> > smaller than RTE_MEMPOOL_CACHE_MAX_SIZE. Let's say the cache size is
> 8,
> > which will make the flush threshold 12. If n is 32, your code will
> not enter this
> > branch, but proceed to call rte_mempool_cache_zc_put_bulk(), which
> will
> > return NULL, and then you will goto done.
> >
> > Obviously, if there is no cache, fall back to the standard
> > rte_mempool_put_bulk().
> 
> Agree with this. I think we ignore the case that (cache -> flushthresh
> < n <  RTE_MEMPOOL_CACHE_MAX_SIZE).
> 
> Our goal is that if (!cache || n > cache -> flushthresh), we can put
> the buffers
> into mempool directly.
> 
> Thus maybe we can change as:
> struct rte_mempool_cache *cache = rte_mempool_default_cache(mp,
> rte_lcore_id());
> if (!cache || n > cache -> flushthresh) {
>       for (i = 0; i < n ; i++)
>           free[i] = txep[i].mbuf;
>       if (!cache) {
>                 rte_mempool_generic_put;
>                 goto done;
>       } else if {
>                 rte_mempool_ops_enqueue_bulk;
>                 goto done;
>       }
> }
> 
> If we can change like this?

Since I consider "flushthreshold" private to the cache structure, it shouldn't be accessed directly. If its meaning changes in the future, you will have to rewrite the PMD code again. Use the mempool API instead of accessing the mempool structures directly. (Yeah, I know the mempool and mempool cache structures are not marked as internal, and thus formally public, but I still dislike accessing their internals from outside the mempool library.)

I would change to something like:

struct rte_mempool_cache *cache;
void **cache_objs;

cache = rte_mempool_default_cache(mp, rte_lcore_id());
if (unlikely(cache == NULL))
	goto fallback;

/* Try zero-copy put. */
cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, n);
if (unlikely(cache_objs == NULL))
	goto fallback;

/* Zero-copy put. */
/* no need to reset txep[i].mbuf in vector path */
for (i = 0; i < n; i++)
	cache_objs[i] = txep[i].mbuf;
goto done;

fallback:
/* Ordinary put. */
/* no need to reset txep[i].mbuf in vector path */
for (i = 0; i < n ; i++)
	free[i] = txep[i].mbuf;
rte_mempool_generic_put(mp, free, n, cache);
goto done;


> 
> >
> > > +			for (i = 0; i < n ; i++)
> > > +				free[i] = txep[i].mbuf;
> > > +			if (!cache) {
> > > +				rte_mempool_generic_put(mp, (void
> > **)free, n,
> > > cache);
> > > +				goto done;
> > > +			}
> > > +			if (n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
> > > +				rte_mempool_ops_enqueue_bulk(mp, (void
> > **)free,
> > > n);
> > > +				goto done;
> > > +			}
> > > +		}
> > > +		void **cache_objs;
> > > +
> > > +		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp,
> > n);
> > > +		if (cache_objs) {
> > > +			for (i = 0; i < n; i++) {
> > > +				cache_objs[i] = txep->mbuf;
> > > +				/* no need to reset txep[i].mbuf in vector
> > path
> > > */
> > > +				txep++;
> > > +			}
> > >  		}
> > > -		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
> > >  		goto done;
> > >  	}
> > >
> > > --
> > > 2.25.1
> > >

回复: [PATCH 1/2] net/i40e: replace put function

[Feifei Wang]

> -----邮件原件-----
> 发件人: Morten Brørup <mb@smartsharesystems.com>
> 发送时间: Thursday, February 9, 2023 7:31 PM
> 收件人: Feifei Wang <Feifei.Wang2@arm.com>; Kamalakshitha Aligeri
> <Kamalakshitha.Aligeri@arm.com>; Yuying.Zhang@intel.com;
> beilei.xing@intel.com; olivier.matz@6wind.com;
> andrew.rybchenko@oktetlabs.ru; bruce.richardson@intel.com;
> konstantin.ananyev@huawei.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>
> 抄送: dev@dpdk.org; nd <nd@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; nd <nd@arm.com>
> 主题: RE: [PATCH 1/2] net/i40e: replace put function
> 
> > From: Feifei Wang [mailto:Feifei.Wang2@arm.com]
> > Sent: Thursday, 9 February 2023 11.59
> >
> > Hi, Morten
> >
> > > 发件人: Morten Brørup <mb@smartsharesystems.com>
> > > 发送时间: Thursday, February 9, 2023 5:34 PM
> > >
> > > > From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> > > > Sent: Thursday, 9 February 2023 07.25
> > > >
> > > > Integrated zero-copy put API in mempool cache in i40e PMD.
> > > > On Ampere Altra server, l3fwd single core's performance improves
> > > > by
> > 5%
> > > > with the new API
> > > >
> > > > Signed-off-by: Kamalakshitha Aligeri
> > <kamalakshitha.aligeri@arm.com>
> > > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> > > > ---
> > > > Link:
> > > >
> https://patchwork.dpdk.org/project/dpdk/patch/20221227151700.80887
> > > > -
> > 1-
> > > > mb@smartsharesystems.com/
> > > >
> > > >  .mailmap                                |  1 +
> > > >  drivers/net/i40e/i40e_rxtx_vec_common.h | 34
> > > > ++++++++++++++++++++-----
> > > >  2 files changed, 28 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/.mailmap b/.mailmap
> > > > index 75884b6fe2..05a42edbcf 100644
> > > > --- a/.mailmap
> > > > +++ b/.mailmap
> > > > @@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>  Kaiwen Deng
> > > > <kaiwenx.deng@intel.com>  Kalesh AP
> > > > <kalesh-anakkur.purayil@broadcom.com>
> > > >  Kamalakannan R <kamalakannan.r@intel.com>
> > > > +Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > > >  Kamil Bednarczyk <kamil.bednarczyk@intel.com>  Kamil Chalupnik
> > > > <kamilx.chalupnik@intel.com>  Kamil Rytarowski
> > > > <kamil.rytarowski@caviumnetworks.com>
> > > > diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h
> > > > b/drivers/net/i40e/i40e_rxtx_vec_common.h
> > > > index fe1a6ec75e..80d4a159e6 100644
> > > > --- a/drivers/net/i40e/i40e_rxtx_vec_common.h
> > > > +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
> > > > @@ -95,17 +95,37 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)
> > > >
> > > >  	n = txq->tx_rs_thresh;
> > > >
> > > > -	 /* first buffer to free from S/W ring is at index
> > > > -	  * tx_next_dd - (tx_rs_thresh-1)
> > > > -	  */
> > > > +	/* first buffer to free from S/W ring is at index
> > > > +	 * tx_next_dd - (tx_rs_thresh-1)
> > > > +	 */
> > > >  	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];
> > > >
> > > >  	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
> > > > -		for (i = 0; i < n; i++) {
> > > > -			free[i] = txep[i].mbuf;
> > > > -			/* no need to reset txep[i].mbuf in vector path */
> > > > +		struct rte_mempool *mp = txep[0].mbuf->pool;
> > > > +		struct rte_mempool_cache *cache =
> > > > rte_mempool_default_cache(mp, rte_lcore_id());
> > > > +
> > > > +		if (!cache || n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
> > >
> > > If the mempool has a cache, do not compare n to
> > > RTE_MEMPOOL_CACHE_MAX_SIZE. Instead, call
> > > rte_mempool_cache_zc_put_bulk() to determine if n is acceptable for
> > zero-
> > > copy.
> > >
> >
> > > It looks like this patch behaves incorrectly if the cache is
> > configured to be
> > > smaller than RTE_MEMPOOL_CACHE_MAX_SIZE. Let's say the cache size
> is
> > 8,
> > > which will make the flush threshold 12. If n is 32, your code will
> > not enter this
> > > branch, but proceed to call rte_mempool_cache_zc_put_bulk(), which
> > will
> > > return NULL, and then you will goto done.
> > >
> > > Obviously, if there is no cache, fall back to the standard
> > > rte_mempool_put_bulk().
> >
> > Agree with this. I think we ignore the case that (cache -> flushthresh
> > < n <  RTE_MEMPOOL_CACHE_MAX_SIZE).
> >
> > Our goal is that if (!cache || n > cache -> flushthresh), we can put
> > the buffers into mempool directly.
> >
> > Thus maybe we can change as:
> > struct rte_mempool_cache *cache = rte_mempool_default_cache(mp,
> > rte_lcore_id()); if (!cache || n > cache -> flushthresh) {
> >       for (i = 0; i < n ; i++)
> >           free[i] = txep[i].mbuf;
> >       if (!cache) {
> >                 rte_mempool_generic_put;
> >                 goto done;
> >       } else if {
> >                 rte_mempool_ops_enqueue_bulk;
> >                 goto done;
> >       }
> > }
> >
> > If we can change like this?
> 
> Since I consider "flushthreshold" private to the cache structure, it shouldn't
> be accessed directly. If its meaning changes in the future, you will have to
> rewrite the PMD code again. 
Ok, Agree with it. Using private variable to the cache needs to be treated with
caution.

Use the mempool API instead of accessing the
> mempool structures directly. (Yeah, I know the mempool and mempool
> cache structures are not marked as internal, and thus formally public, but I
> still dislike accessing their internals from outside the mempool library.)
> 
> I would change to something like:
> 
> struct rte_mempool_cache *cache;
> void **cache_objs;
> 
> cache = rte_mempool_default_cache(mp, rte_lcore_id()); if (unlikely(cache
> == NULL))
> 	goto fallback;
> 
> /* Try zero-copy put. */
> cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, n); if
> (unlikely(cache_objs == NULL))
> 	goto fallback;
> 
> /* Zero-copy put. */
> /* no need to reset txep[i].mbuf in vector path */ for (i = 0; i < n; i++)
> 	cache_objs[i] = txep[i].mbuf;
> goto done;
> 
> fallback:
> /* Ordinary put. */
> /* no need to reset txep[i].mbuf in vector path */ 
This note should be deleted, due to here we need to reset txep[i].mbuf.

for (i = 0; i < n ; i++)
> 	free[i] = txep[i].mbuf;
> rte_mempool_generic_put(mp, free, n, cache); goto done;
> 
> 
Agree with this change, some minor comments for the notes in 'fallback'.

> >
> > >
> > > > +			for (i = 0; i < n ; i++)
> > > > +				free[i] = txep[i].mbuf;
> > > > +			if (!cache) {
> > > > +				rte_mempool_generic_put(mp, (void
> > > **)free, n,
> > > > cache);
> > > > +				goto done;
> > > > +			}
> > > > +			if (n > RTE_MEMPOOL_CACHE_MAX_SIZE) {
> > > > +				rte_mempool_ops_enqueue_bulk(mp, (void
> > > **)free,
> > > > n);
> > > > +				goto done;
> > > > +			}
> > > > +		}
> > > > +		void **cache_objs;
> > > > +
> > > > +		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp,
> > > n);
> > > > +		if (cache_objs) {
> > > > +			for (i = 0; i < n; i++) {
> > > > +				cache_objs[i] = txep->mbuf;
> > > > +				/* no need to reset txep[i].mbuf in vector
> > > path
> > > > */
> > > > +				txep++;
> > > > +			}
> > > >  		}
> > > > -		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
> > > >  		goto done;
> > > >  	}
> > > >
> > > > --
> > > > 2.25.1
> > > >

[PATCH v2 0/2] Integrated mempool cache zero-copy API's

[Kamalakshitha Aligeri]

In i40e PMD, for fast_free path in Tx, freeing mbufs need to do an extra
memcpy due to the special structure of ‘txep’.
Thus, replaced the put_bulk function with the zero-copy put API to
avoid the cost of this memcpy.
On Ampere-altra server, for single-core DPDK l3fwd test, throughput
improves by 5% with the new API

v2:
Fixed the code for n > RTE_MEMPOOL_CACHE_MAX_SIZE (Morten Brorup)
v1:
1. Integrated the zc_put API in i40e PMD
2. Added mempool test cases with the zero-cpoy API's

Kamalakshitha Aligeri (2):
  net/i40e: replace put function
  test/mempool: add zero-copy API's

 .mailmap                                |  1 +
 app/test/test_mempool.c                 | 81 ++++++++++++++++++-------
 drivers/net/i40e/i40e_rxtx_vec_common.h | 28 +++++++--
 3 files changed, 84 insertions(+), 26 deletions(-)

--
2.25.1

[PATCH v2 1/2] net/i40e: replace put function

[Kamalakshitha Aligeri]

Integrated zero-copy put API in mempool cache in i40e PMD.
On Ampere Altra server, l3fwd single core's performance improves by 5%
with the new API

Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
Link: https://patchwork.dpdk.org/project/dpdk/patch/20230209145833.129986-1-mb@smartsharesystems.com/

 .mailmap                                |  1 +
 drivers/net/i40e/i40e_rxtx_vec_common.h | 28 ++++++++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/.mailmap b/.mailmap
index 75884b6fe2..05a42edbcf 100644
--- a/.mailmap
+++ b/.mailmap
@@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>
 Kaiwen Deng <kaiwenx.deng@intel.com>
 Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
 Kamalakannan R <kamalakannan.r@intel.com>
+Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
 Kamil Bednarczyk <kamil.bednarczyk@intel.com>
 Kamil Chalupnik <kamilx.chalupnik@intel.com>
 Kamil Rytarowski <kamil.rytarowski@caviumnetworks.com>
diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h
index fe1a6ec75e..113599d82b 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_common.h
+++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
@@ -95,18 +95,36 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)

 	n = txq->tx_rs_thresh;

-	 /* first buffer to free from S/W ring is at index
-	  * tx_next_dd - (tx_rs_thresh-1)
-	  */
+	/* first buffer to free from S/W ring is at index
+	 * tx_next_dd - (tx_rs_thresh-1)
+	 */
 	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];

 	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
+		struct rte_mempool *mp = txep[0].mbuf->pool;
+		struct rte_mempool_cache *cache = rte_mempool_default_cache(mp, rte_lcore_id());
+		void **cache_objs;
+
+		if (unlikely(!cache))
+			goto fallback;
+
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, n);
+		if (unlikely(!cache_objs))
+			goto fallback;
+
 		for (i = 0; i < n; i++) {
-			free[i] = txep[i].mbuf;
+			cache_objs[i] = txep->mbuf;
 			/* no need to reset txep[i].mbuf in vector path */
+			txep++;
 		}
-		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
 		goto done;
+
+fallback:
+		for (i = 0; i < n; i++)
+			free[i] = txep[i].mbuf;
+		rte_mempool_generic_put(mp, (void **)free, n, cache);
+		goto done;
+
 	}

 	m = rte_pktmbuf_prefree_seg(txep[0].mbuf);
--
2.25.1

RE: [PATCH v2 1/2] net/i40e: replace put function

[Morten Brørup]

> From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> Sent: Friday, 10 February 2023 07.54
> 
> Integrated zero-copy put API in mempool cache in i40e PMD.
> On Ampere Altra server, l3fwd single core's performance improves by 5%
> with the new API
> 
> Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> ---
> Link:
> https://patchwork.dpdk.org/project/dpdk/patch/20230209145833.129986-1-
> mb@smartsharesystems.com/

If you agree with the referred patch, please review or acknowledge it on the mailing list, so it can be merged.

> 
>  .mailmap                                |  1 +
>  drivers/net/i40e/i40e_rxtx_vec_common.h | 28 ++++++++++++++++++++-----
>  2 files changed, 24 insertions(+), 5 deletions(-)
> 
> diff --git a/.mailmap b/.mailmap
> index 75884b6fe2..05a42edbcf 100644
> --- a/.mailmap
> +++ b/.mailmap
> @@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>
>  Kaiwen Deng <kaiwenx.deng@intel.com>
>  Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>  Kamalakannan R <kamalakannan.r@intel.com>
> +Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
>  Kamil Bednarczyk <kamil.bednarczyk@intel.com>
>  Kamil Chalupnik <kamilx.chalupnik@intel.com>
>  Kamil Rytarowski <kamil.rytarowski@caviumnetworks.com>
> diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h
> b/drivers/net/i40e/i40e_rxtx_vec_common.h
> index fe1a6ec75e..113599d82b 100644
> --- a/drivers/net/i40e/i40e_rxtx_vec_common.h
> +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
> @@ -95,18 +95,36 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)
> 
>  	n = txq->tx_rs_thresh;
> 
> -	 /* first buffer to free from S/W ring is at index
> -	  * tx_next_dd - (tx_rs_thresh-1)
> -	  */
> +	/* first buffer to free from S/W ring is at index
> +	 * tx_next_dd - (tx_rs_thresh-1)
> +	 */
>  	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];
> 
>  	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
> +		struct rte_mempool *mp = txep[0].mbuf->pool;
> +		struct rte_mempool_cache *cache =
> rte_mempool_default_cache(mp, rte_lcore_id());
> +		void **cache_objs;
> +
> +		if (unlikely(!cache))
> +			goto fallback;
> +
> +		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, n);
> +		if (unlikely(!cache_objs))
> +			goto fallback;
> +
>  		for (i = 0; i < n; i++) {
> -			free[i] = txep[i].mbuf;
> +			cache_objs[i] = txep->mbuf;
>  			/* no need to reset txep[i].mbuf in vector path */
> +			txep++;

Why the change from "xyz[i] = txep[i].mbuf;" to "xyz[i] = txep->mbuf; txep++;"? I don't see "txep" being used after the "done" label. And at the fallback, you still use "xyz[i] = txep[i].mbuf;". It would look cleaner using the same method in both places.

It's not important, so feel free to keep as is or change as suggested. Both ways,

Acked-by: Morten Brørup <mb@smartsharesystems.com>

>  		}
> -		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
>  		goto done;
> +
> +fallback:
> +		for (i = 0; i < n; i++)
> +			free[i] = txep[i].mbuf;
> +		rte_mempool_generic_put(mp, (void **)free, n, cache);
> +		goto done;
> +
>  	}
> 
>  	m = rte_pktmbuf_prefree_seg(txep[0].mbuf);
> --
> 2.25.1
> 

RE: [PATCH v2 1/2] net/i40e: replace put function

[Honnappa Nagarahalli]

<snip>
[
> 
> > From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> > Sent: Friday, 10 February 2023 07.54
> >
> > Integrated zero-copy put API in mempool cache in i40e PMD.
> > On Ampere Altra server, l3fwd single core's performance improves by 5%
> > with the new API
> >
> > Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> > ---
> > Link:
> > https://patchwork.dpdk.org/project/dpdk/patch/20230209145833.129986-1-
> > mb@smartsharesystems.com/
> 
> If you agree with the referred patch, please review or acknowledge it on the
> mailing list, so it can be merged.
> 
> >
> >  .mailmap                                |  1 +
> >  drivers/net/i40e/i40e_rxtx_vec_common.h | 28
> > ++++++++++++++++++++-----
> >  2 files changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/.mailmap b/.mailmap
> > index 75884b6fe2..05a42edbcf 100644
> > --- a/.mailmap
> > +++ b/.mailmap
> > @@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>  Kaiwen Deng
> > <kaiwenx.deng@intel.com>  Kalesh AP
> > <kalesh-anakkur.purayil@broadcom.com>
> >  Kamalakannan R <kamalakannan.r@intel.com>
> > +Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> >  Kamil Bednarczyk <kamil.bednarczyk@intel.com>  Kamil Chalupnik
> > <kamilx.chalupnik@intel.com>  Kamil Rytarowski
> > <kamil.rytarowski@caviumnetworks.com>
> > diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h
> > b/drivers/net/i40e/i40e_rxtx_vec_common.h
> > index fe1a6ec75e..113599d82b 100644
> > --- a/drivers/net/i40e/i40e_rxtx_vec_common.h
> > +++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
> > @@ -95,18 +95,36 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)
> >
> >  	n = txq->tx_rs_thresh;
> >
> > -	 /* first buffer to free from S/W ring is at index
> > -	  * tx_next_dd - (tx_rs_thresh-1)
> > -	  */
> > +	/* first buffer to free from S/W ring is at index
> > +	 * tx_next_dd - (tx_rs_thresh-1)
> > +	 */
> >  	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];
> >
> >  	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
> > +		struct rte_mempool *mp = txep[0].mbuf->pool;
> > +		struct rte_mempool_cache *cache =
> > rte_mempool_default_cache(mp, rte_lcore_id());
> > +		void **cache_objs;
> > +
> > +		if (unlikely(!cache))
> > +			goto fallback;
> > +
> > +		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, n);
> > +		if (unlikely(!cache_objs))
> > +			goto fallback;
> > +
> >  		for (i = 0; i < n; i++) {
> > -			free[i] = txep[i].mbuf;
> > +			cache_objs[i] = txep->mbuf;
> >  			/* no need to reset txep[i].mbuf in vector path */
> > +			txep++;
> 
> Why the change from "xyz[i] = txep[i].mbuf;" to "xyz[i] = txep->mbuf; txep++;"? I
> don't see "txep" being used after the "done" label. And at the fallback, you still
> use "xyz[i] = txep[i].mbuf;". It would look cleaner using the same method in
> both places.
+1

> 
> It's not important, so feel free to keep as is or change as suggested. Both ways,
> 
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 
> >  		}
> > -		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
> >  		goto done;
> > +
> > +fallback:
> > +		for (i = 0; i < n; i++)
> > +			free[i] = txep[i].mbuf;
> > +		rte_mempool_generic_put(mp, (void **)free, n, cache);
> > +		goto done;
> > +
> >  	}
> >
> >  	m = rte_pktmbuf_prefree_seg(txep[0].mbuf);
> > --
> > 2.25.1
> >

[PATCH v3 0/2] Integrated mempool cache zero-copy API's

[Kamalakshitha Aligeri]

In i40e PMD, for fast_free path in Tx, freeing mbufs firstly need to do
an extra memcpy due to the special structure of ‘txep’.
Thus, replaced the put_bulk function with the zero-copy put API to
avoid the cost of this memcpy
On Ampere-altra server, for single-core DPDK l3fwd test, throughput
improves by 5% with the new API

v3:
Fixed the way mbufs are accessed from txep (Morten Brorup)
v2:
Fixed the code for n > RTE_MEMPOOL_CACHE_MAX_SIZE (Morten Brorup)
v1:
1. Integrated the zc_put API in i40e PMD
2. Added mempool test cases with the zero-cpoy API's

Kamalakshitha Aligeri (2):
  net/i40e: replace put function
  test/mempool: add zero-copy API's

 .mailmap                                |  1 +
 app/test/test_mempool.c                 | 81 ++++++++++++++++++-------
 drivers/net/i40e/i40e_rxtx_vec_common.h | 27 +++++++--
 3 files changed, 83 insertions(+), 26 deletions(-)

--
2.25.1

[PATCH v3 1/2] net/i40e: replace put function

[Kamalakshitha Aligeri]

Integrated zero-copy put API in mempool cache in i40e PMD.
On Ampere Altra server, l3fwd single core's performance improves by 5%
with the new API

Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
Link: http://patches.dpdk.org/project/dpdk/patch/20230213122437.122858-1-mb@smartsharesystems.com/

 .mailmap                                |  1 +
 drivers/net/i40e/i40e_rxtx_vec_common.h | 27 ++++++++++++++++++++-----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/.mailmap b/.mailmap
index 75884b6fe2..05a42edbcf 100644
--- a/.mailmap
+++ b/.mailmap
@@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>
 Kaiwen Deng <kaiwenx.deng@intel.com>
 Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
 Kamalakannan R <kamalakannan.r@intel.com>
+Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
 Kamil Bednarczyk <kamil.bednarczyk@intel.com>
 Kamil Chalupnik <kamilx.chalupnik@intel.com>
 Kamil Rytarowski <kamil.rytarowski@caviumnetworks.com>
diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h
index fe1a6ec75e..35cdb31b2e 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_common.h
+++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
@@ -95,18 +95,35 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)

 	n = txq->tx_rs_thresh;

-	 /* first buffer to free from S/W ring is at index
-	  * tx_next_dd - (tx_rs_thresh-1)
-	  */
+	/* first buffer to free from S/W ring is at index
+	 * tx_next_dd - (tx_rs_thresh-1)
+	 */
 	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];

 	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
+		struct rte_mempool *mp = txep[0].mbuf->pool;
+		struct rte_mempool_cache *cache = rte_mempool_default_cache(mp, rte_lcore_id());
+		void **cache_objs;
+
+		if (unlikely(!cache))
+			goto fallback;
+
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, n);
+		if (unlikely(!cache_objs))
+			goto fallback;
+
 		for (i = 0; i < n; i++) {
-			free[i] = txep[i].mbuf;
+			cache_objs[i] = txep[i].mbuf;
 			/* no need to reset txep[i].mbuf in vector path */
 		}
-		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
 		goto done;
+
+fallback:
+		for (i = 0; i < n; i++)
+			free[i] = txep[i].mbuf;
+		rte_mempool_generic_put(mp, (void **)free, n, cache);
+		goto done;
+
 	}

 	m = rte_pktmbuf_prefree_seg(txep[0].mbuf);
--
2.25.1

RE: [PATCH v3 1/2] net/i40e: replace put function

[Morten Brørup]

> From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> Sent: Monday, 13 February 2023 19.18
> 
> Integrated zero-copy put API in mempool cache in i40e PMD.
> On Ampere Altra server, l3fwd single core's performance improves by 5%
> with the new API
> 
> Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> ---
> Link: http://patches.dpdk.org/project/dpdk/patch/20230213122437.122858-
> 1-mb@smartsharesystems.com/

Reviewed-by: Morten Brørup <mb@smartsharesystems.com>

RE: [PATCH v3 1/2] net/i40e: replace put function

[Lu, Wenzhuo]

Hi Kamalakshitha,

> -----Original Message-----
> From: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> Sent: Tuesday, February 14, 2023 2:18 AM
> To: Zhang, Yuying <yuying.zhang@intel.com>; Xing, Beilei
> <beilei.xing@intel.com>; Matz, Olivier <olivier.matz@6wind.com>;
> andrew.rybchenko@oktetlabs.ru; Richardson, Bruce
> <bruce.richardson@intel.com>; mb@smartsharesystems.com;
> konstantin.ananyev@huawei.com; Honnappa.Nagarahalli@arm.com;
> ruifeng.wang@arm.com; feifei.wang2@arm.com
> Cc: dev@dpdk.org; nd@arm.com; Kamalakshitha Aligeri
> <kamalakshitha.aligeri@arm.com>
> Subject: [PATCH v3 1/2] net/i40e: replace put function
> 
> Integrated zero-copy put API in mempool cache in i40e PMD.
> On Ampere Altra server, l3fwd single core's performance improves by 5% with
> the new API
> 
> Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
We're trying to check the performance improvement. But met compile failure. The same as CI reported, http://mails.dpdk.org/archives/test-report/2023-February/353245.html.

RE: [PATCH v3 1/2] net/i40e: replace put function

[Morten Brørup]

> From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]
> Sent: Friday, 17 February 2023 03.03
> 
> Hi Kamalakshitha,
> 
> > From: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > Sent: Tuesday, February 14, 2023 2:18 AM
> >
> > Integrated zero-copy put API in mempool cache in i40e PMD.
> > On Ampere Altra server, l3fwd single core's performance improves by
> 5% with
> > the new API
> >
> > Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> We're trying to check the performance improvement. But met compile
> failure. The same as CI reported, http://mails.dpdk.org/archives/test-
> report/2023-February/353245.html.

It depends on this patch:
https://patchwork.dpdk.org/project/dpdk/patch/20230213122437.122858-1-mb@smartsharesystems.com/

Does anyone know how to formally add such a dependency to the patch description, so the CI can understand it?

Re: [PATCH v3 1/2] net/i40e: replace put function

[Ferruh Yigit]

On 2/17/2023 7:52 AM, Morten Brørup wrote:
>> From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]
>> Sent: Friday, 17 February 2023 03.03
>>
>> Hi Kamalakshitha,
>>
>>> From: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
>>> Sent: Tuesday, February 14, 2023 2:18 AM
>>>
>>> Integrated zero-copy put API in mempool cache in i40e PMD.
>>> On Ampere Altra server, l3fwd single core's performance improves by
>> 5% with
>>> the new API
>>>
>>> Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>>> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
>> We're trying to check the performance improvement. But met compile
>> failure. The same as CI reported, http://mails.dpdk.org/archives/test-
>> report/2023-February/353245.html.
> 
> It depends on this patch:
> https://patchwork.dpdk.org/project/dpdk/patch/20230213122437.122858-1-mb@smartsharesystems.com/
> 
> Does anyone know how to formally add such a dependency to the patch description, so the CI can understand it?
> 

Hi Morten,

This has been discussed in the past and a syntax defined:
https://doc.dpdk.org/guides/contributing/patches.html#patch-dependencies

But the problem is CI not implemented it yet, it was in the CI backlog
last time I remember but I don't know current status. @Aaron may know.

RE: [PATCH v3 1/2] net/i40e: replace put function

[Morten Brørup]

> From: Ferruh Yigit [mailto:ferruh.yigit@amd.com]
> Sent: Friday, 17 February 2023 11.05
> 
> On 2/17/2023 7:52 AM, Morten Brørup wrote:
> >> From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]
> >> Sent: Friday, 17 February 2023 03.03
> >>
> >> Hi Kamalakshitha,
> >>
> >>> From: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> >>> Sent: Tuesday, February 14, 2023 2:18 AM
> >>>
> >>> Integrated zero-copy put API in mempool cache in i40e PMD.
> >>> On Ampere Altra server, l3fwd single core's performance improves by
> >> 5% with
> >>> the new API
> >>>
> >>> Signed-off-by: Kamalakshitha Aligeri
> <kamalakshitha.aligeri@arm.com>
> >>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> >>> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> >> We're trying to check the performance improvement. But met compile
> >> failure. The same as CI reported,
> http://mails.dpdk.org/archives/test-
> >> report/2023-February/353245.html.
> >
> > It depends on this patch:
> > https://patchwork.dpdk.org/project/dpdk/patch/20230213122437.122858-
> 1-mb@smartsharesystems.com/
> >
> > Does anyone know how to formally add such a dependency to the patch
> description, so the CI can understand it?
> >
> 
> Hi Morten,
> 
> This has been discussed in the past and a syntax defined:
> https://doc.dpdk.org/guides/contributing/patches.html#patch-
> dependencies

So for this patch, Kamalakshitha should add the tag:

Depends-on: series-26984 ("[v9] mempool cache: add zero-copy get and put functions")

> 
> But the problem is CI not implemented it yet, it was in the CI backlog
> last time I remember but I don't know current status. @Aaron may know.

Re: [PATCH v3 1/2] net/i40e: replace put function

[Aaron Conole]

Morten Brørup <mb@smartsharesystems.com> writes:

>> From: Ferruh Yigit [mailto:ferruh.yigit@amd.com]
>> Sent: Friday, 17 February 2023 11.05
>> 
>> On 2/17/2023 7:52 AM, Morten Brørup wrote:
>> >> From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]
>> >> Sent: Friday, 17 February 2023 03.03
>> >>
>> >> Hi Kamalakshitha,
>> >>
>> >>> From: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
>> >>> Sent: Tuesday, February 14, 2023 2:18 AM
>> >>>
>> >>> Integrated zero-copy put API in mempool cache in i40e PMD.
>> >>> On Ampere Altra server, l3fwd single core's performance improves by
>> >> 5% with
>> >>> the new API
>> >>>
>> >>> Signed-off-by: Kamalakshitha Aligeri
>> <kamalakshitha.aligeri@arm.com>
>> >>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>> >>> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
>> >> We're trying to check the performance improvement. But met compile
>> >> failure. The same as CI reported,
>> http://mails.dpdk.org/archives/test-
>> >> report/2023-February/353245.html.
>> >
>> > It depends on this patch:
>> > https://patchwork.dpdk.org/project/dpdk/patch/20230213122437.122858-
>> 1-mb@smartsharesystems.com/
>> >
>> > Does anyone know how to formally add such a dependency to the patch
>> description, so the CI can understand it?
>> >
>> 
>> Hi Morten,
>> 
>> This has been discussed in the past and a syntax defined:
>> https://doc.dpdk.org/guides/contributing/patches.html#patch-
>> dependencies
>
> So for this patch, Kamalakshitha should add the tag:
>
> Depends-on: series-26984 ("[v9] mempool cache: add zero-copy get and put functions")

I think just:

Depends-on: series-26984

In the first patch, or cover letter.  That said, I did a quick test and
the 0day bot should support it now.

>> 
>> But the problem is CI not implemented it yet, it was in the CI backlog
>> last time I remember but I don't know current status. @Aaron may know.

Re: [PATCH v3 1/2] net/i40e: replace put function

[Aaron Conole]

Ferruh Yigit <ferruh.yigit@amd.com> writes:

> On 2/17/2023 7:52 AM, Morten Brørup wrote:
>>> From: Lu, Wenzhuo [mailto:wenzhuo.lu@intel.com]
>>> Sent: Friday, 17 February 2023 03.03
>>>
>>> Hi Kamalakshitha,
>>>
>>>> From: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
>>>> Sent: Tuesday, February 14, 2023 2:18 AM
>>>>
>>>> Integrated zero-copy put API in mempool cache in i40e PMD.
>>>> On Ampere Altra server, l3fwd single core's performance improves by
>>> 5% with
>>>> the new API
>>>>
>>>> Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>>>> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
>>> We're trying to check the performance improvement. But met compile
>>> failure. The same as CI reported, http://mails.dpdk.org/archives/test-
>>> report/2023-February/353245.html.
>> 
>> It depends on this patch:
>> https://patchwork.dpdk.org/project/dpdk/patch/20230213122437.122858-1-mb@smartsharesystems.com/
>> 
>> Does anyone know how to formally add such a dependency to the patch
>> description, so the CI can understand it?
>> 
>
> Hi Morten,
>
> This has been discussed in the past and a syntax defined:
> https://doc.dpdk.org/guides/contributing/patches.html#patch-dependencies
>
> But the problem is CI not implemented it yet, it was in the CI backlog
> last time I remember but I don't know current status. @Aaron may know.

I believe it is implemented in 0day and UNH bots.  Not 100% certain on
the UNH side.

[PATCH v3 2/2] test/mempool: add zero-copy API's

[Kamalakshitha Aligeri]

Added mempool test cases with zero-copy get and put API's

Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
Link: http://patches.dpdk.org/project/dpdk/patch/20230213122437.122858-1-mb@smartsharesystems.com/

 app/test/test_mempool.c | 81 ++++++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 21 deletions(-)

diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 8e493eda47..6d29f5bc7b 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -74,7 +74,7 @@ my_obj_init(struct rte_mempool *mp, __rte_unused void *arg,

 /* basic tests (done on one core) */
 static int
-test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
+test_mempool_basic(struct rte_mempool *mp, int use_external_cache, int use_zc_api)
 {
 	uint32_t *objnum;
 	void **objtable;
@@ -84,6 +84,7 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 	unsigned i, j;
 	int offset;
 	struct rte_mempool_cache *cache;
+	void **cache_objs;

 	if (use_external_cache) {
 		/* Create a user-owned mempool cache. */
@@ -100,8 +101,13 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 	rte_mempool_dump(stdout, mp);

 	printf("get an object\n");
-	if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
-		GOTO_ERR(ret, out);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj = *cache_objs;
+	} else {
+		if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
+			GOTO_ERR(ret, out);
+	}
 	rte_mempool_dump(stdout, mp);

 	/* tests that improve coverage */
@@ -123,21 +129,41 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 #endif

 	printf("put the object back\n");
-	rte_mempool_generic_put(mp, &obj, 1, cache);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj, sizeof(void *));
+	} else {
+		rte_mempool_generic_put(mp, &obj, 1, cache);
+	}
 	rte_mempool_dump(stdout, mp);

 	printf("get 2 objects\n");
-	if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
-		GOTO_ERR(ret, out);
-	if (rte_mempool_generic_get(mp, &obj2, 1, cache) < 0) {
-		rte_mempool_generic_put(mp, &obj, 1, cache);
-		GOTO_ERR(ret, out);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj = *cache_objs;
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj2 = *cache_objs;
+	} else {
+		if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
+			GOTO_ERR(ret, out);
+		if (rte_mempool_generic_get(mp, &obj2, 1, cache) < 0) {
+			rte_mempool_generic_put(mp, &obj, 1, cache);
+			GOTO_ERR(ret, out);
+		}
 	}
 	rte_mempool_dump(stdout, mp);

 	printf("put the objects back\n");
-	rte_mempool_generic_put(mp, &obj, 1, cache);
-	rte_mempool_generic_put(mp, &obj2, 1, cache);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj, sizeof(void *));
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj2, sizeof(void *));
+
+	} else {
+		rte_mempool_generic_put(mp, &obj, 1, cache);
+		rte_mempool_generic_put(mp, &obj2, 1, cache);
+	}
 	rte_mempool_dump(stdout, mp);

 	/*
@@ -149,8 +175,13 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 		GOTO_ERR(ret, out);

 	for (i = 0; i < MEMPOOL_SIZE; i++) {
-		if (rte_mempool_generic_get(mp, &objtable[i], 1, cache) < 0)
-			break;
+		if (use_zc_api) {
+			cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+			objtable[i] = *cache_objs;
+		} else {
+			if (rte_mempool_generic_get(mp, &objtable[i], 1, cache) < 0)
+				break;
+		}
 	}

 	/*
@@ -170,8 +201,12 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 			if (obj_data[j] != 0)
 				ret = -1;
 		}
-
-		rte_mempool_generic_put(mp, &objtable[i], 1, cache);
+		if (use_zc_api) {
+			cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+			rte_memcpy(cache_objs, &objtable[i], sizeof(void *));
+		} else {
+			rte_mempool_generic_put(mp, &objtable[i], 1, cache);
+		}
 	}

 	free(objtable);
@@ -979,15 +1014,19 @@ test_mempool(void)
 	rte_mempool_list_dump(stdout);

 	/* basic tests without cache */
-	if (test_mempool_basic(mp_nocache, 0) < 0)
+	if (test_mempool_basic(mp_nocache, 0, 0) < 0)
+		GOTO_ERR(ret, err);
+
+	/* basic tests with zero-copy API's */
+	if (test_mempool_basic(mp_cache, 0, 1) < 0)
 		GOTO_ERR(ret, err);

-	/* basic tests with cache */
-	if (test_mempool_basic(mp_cache, 0) < 0)
+	/* basic tests with user-owned cache and zero-copy API's */
+	if (test_mempool_basic(mp_nocache, 1, 1) < 0)
 		GOTO_ERR(ret, err);

 	/* basic tests with user-owned cache */
-	if (test_mempool_basic(mp_nocache, 1) < 0)
+	if (test_mempool_basic(mp_nocache, 1, 0) < 0)
 		GOTO_ERR(ret, err);

 	/* more basic tests without cache */
@@ -1008,10 +1047,10 @@ test_mempool(void)
 		GOTO_ERR(ret, err);

 	/* test the stack handler */
-	if (test_mempool_basic(mp_stack, 1) < 0)
+	if (test_mempool_basic(mp_stack, 1, 0) < 0)
 		GOTO_ERR(ret, err);

-	if (test_mempool_basic(default_pool, 1) < 0)
+	if (test_mempool_basic(default_pool, 1, 0) < 0)
 		GOTO_ERR(ret, err);

 	/* test mempool event callbacks */
--
2.25.1

[PATCH v4 0/2] Integrated mempool cache zero-copy API's

[Kamalakshitha Aligeri]

In i40e PMD, for fast_free path in Tx, freeing mbufs firstly need to do
an extra memcpy due to the special structure of ‘txep’.
Thus, replaced the put_bulk function with the zero-copy put API to
avoid the cost of this memcpy
On Ampere-altra server, for single-core DPDK l3fwd test, throughput
improves by 5% with the new API

v4:
Added the Depends-on tag
v3:
Fixed the way mbufs are accessed from txep (Morten Brorup)
v2:
Fixed the code for n > RTE_MEMPOOL_CACHE_MAX_SIZE (Morten Brorup)
v1:
1. Integrated the zc_put API in i40e PMD
2. Added mempool test cases with the zero-cpoy API's

Kamalakshitha Aligeri (2):
  net/i40e: replace put function
  test/mempool: add zero-copy API's

 .mailmap                                |  1 +
 app/test/test_mempool.c                 | 81 ++++++++++++++++++-------
 drivers/net/i40e/i40e_rxtx_vec_common.h | 27 +++++++--
 3 files changed, 83 insertions(+), 26 deletions(-)

--
2.25.1

[PATCH v4 1/2] net/i40e: replace put function

[Kamalakshitha Aligeri]

Integrated zero-copy put API in mempool cache in i40e PMD.
On Ampere Altra server, l3fwd single core's performance improves by 5%
with the new API

Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
Reviewed-by: Morten Brørup <mb@smartsharesystems.com>
---
Depends-on: series-26984 ("[v9] mempool cache: add zero-copy get and put functions")

 .mailmap                                |  1 +
 drivers/net/i40e/i40e_rxtx_vec_common.h | 27 ++++++++++++++++++++-----
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/.mailmap b/.mailmap
index 75884b6fe2..05a42edbcf 100644
--- a/.mailmap
+++ b/.mailmap
@@ -670,6 +670,7 @@ Kai Ji <kai.ji@intel.com>
 Kaiwen Deng <kaiwenx.deng@intel.com>
 Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
 Kamalakannan R <kamalakannan.r@intel.com>
+Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
 Kamil Bednarczyk <kamil.bednarczyk@intel.com>
 Kamil Chalupnik <kamilx.chalupnik@intel.com>
 Kamil Rytarowski <kamil.rytarowski@caviumnetworks.com>
diff --git a/drivers/net/i40e/i40e_rxtx_vec_common.h b/drivers/net/i40e/i40e_rxtx_vec_common.h
index fe1a6ec75e..35cdb31b2e 100644
--- a/drivers/net/i40e/i40e_rxtx_vec_common.h
+++ b/drivers/net/i40e/i40e_rxtx_vec_common.h
@@ -95,18 +95,35 @@ i40e_tx_free_bufs(struct i40e_tx_queue *txq)

 	n = txq->tx_rs_thresh;

-	 /* first buffer to free from S/W ring is at index
-	  * tx_next_dd - (tx_rs_thresh-1)
-	  */
+	/* first buffer to free from S/W ring is at index
+	 * tx_next_dd - (tx_rs_thresh-1)
+	 */
 	txep = &txq->sw_ring[txq->tx_next_dd - (n - 1)];

 	if (txq->offloads & RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE) {
+		struct rte_mempool *mp = txep[0].mbuf->pool;
+		struct rte_mempool_cache *cache = rte_mempool_default_cache(mp, rte_lcore_id());
+		void **cache_objs;
+
+		if (unlikely(!cache))
+			goto fallback;
+
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, n);
+		if (unlikely(!cache_objs))
+			goto fallback;
+
 		for (i = 0; i < n; i++) {
-			free[i] = txep[i].mbuf;
+			cache_objs[i] = txep[i].mbuf;
 			/* no need to reset txep[i].mbuf in vector path */
 		}
-		rte_mempool_put_bulk(free[0]->pool, (void **)free, n);
 		goto done;
+
+fallback:
+		for (i = 0; i < n; i++)
+			free[i] = txep[i].mbuf;
+		rte_mempool_generic_put(mp, (void **)free, n, cache);
+		goto done;
+
 	}

 	m = rte_pktmbuf_prefree_seg(txep[0].mbuf);
--
2.25.1

[PATCH v4 2/2] test/mempool: add zero-copy API's

[Kamalakshitha Aligeri]

Added mempool test cases with zero-copy get and put API's

Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>
---
Depends-on: series-26984 ("[v9] mempool cache: add zero-copy get and put functions")

 app/test/test_mempool.c | 81 ++++++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 21 deletions(-)

diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 8e493eda47..6d29f5bc7b 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -74,7 +74,7 @@ my_obj_init(struct rte_mempool *mp, __rte_unused void *arg,

 /* basic tests (done on one core) */
 static int
-test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
+test_mempool_basic(struct rte_mempool *mp, int use_external_cache, int use_zc_api)
 {
 	uint32_t *objnum;
 	void **objtable;
@@ -84,6 +84,7 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 	unsigned i, j;
 	int offset;
 	struct rte_mempool_cache *cache;
+	void **cache_objs;

 	if (use_external_cache) {
 		/* Create a user-owned mempool cache. */
@@ -100,8 +101,13 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 	rte_mempool_dump(stdout, mp);

 	printf("get an object\n");
-	if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
-		GOTO_ERR(ret, out);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj = *cache_objs;
+	} else {
+		if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
+			GOTO_ERR(ret, out);
+	}
 	rte_mempool_dump(stdout, mp);

 	/* tests that improve coverage */
@@ -123,21 +129,41 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 #endif

 	printf("put the object back\n");
-	rte_mempool_generic_put(mp, &obj, 1, cache);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj, sizeof(void *));
+	} else {
+		rte_mempool_generic_put(mp, &obj, 1, cache);
+	}
 	rte_mempool_dump(stdout, mp);

 	printf("get 2 objects\n");
-	if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
-		GOTO_ERR(ret, out);
-	if (rte_mempool_generic_get(mp, &obj2, 1, cache) < 0) {
-		rte_mempool_generic_put(mp, &obj, 1, cache);
-		GOTO_ERR(ret, out);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj = *cache_objs;
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj2 = *cache_objs;
+	} else {
+		if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
+			GOTO_ERR(ret, out);
+		if (rte_mempool_generic_get(mp, &obj2, 1, cache) < 0) {
+			rte_mempool_generic_put(mp, &obj, 1, cache);
+			GOTO_ERR(ret, out);
+		}
 	}
 	rte_mempool_dump(stdout, mp);

 	printf("put the objects back\n");
-	rte_mempool_generic_put(mp, &obj, 1, cache);
-	rte_mempool_generic_put(mp, &obj2, 1, cache);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj, sizeof(void *));
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj2, sizeof(void *));
+
+	} else {
+		rte_mempool_generic_put(mp, &obj, 1, cache);
+		rte_mempool_generic_put(mp, &obj2, 1, cache);
+	}
 	rte_mempool_dump(stdout, mp);

 	/*
@@ -149,8 +175,13 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 		GOTO_ERR(ret, out);

 	for (i = 0; i < MEMPOOL_SIZE; i++) {
-		if (rte_mempool_generic_get(mp, &objtable[i], 1, cache) < 0)
-			break;
+		if (use_zc_api) {
+			cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+			objtable[i] = *cache_objs;
+		} else {
+			if (rte_mempool_generic_get(mp, &objtable[i], 1, cache) < 0)
+				break;
+		}
 	}

 	/*
@@ -170,8 +201,12 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 			if (obj_data[j] != 0)
 				ret = -1;
 		}
-
-		rte_mempool_generic_put(mp, &objtable[i], 1, cache);
+		if (use_zc_api) {
+			cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+			rte_memcpy(cache_objs, &objtable[i], sizeof(void *));
+		} else {
+			rte_mempool_generic_put(mp, &objtable[i], 1, cache);
+		}
 	}

 	free(objtable);
@@ -979,15 +1014,19 @@ test_mempool(void)
 	rte_mempool_list_dump(stdout);

 	/* basic tests without cache */
-	if (test_mempool_basic(mp_nocache, 0) < 0)
+	if (test_mempool_basic(mp_nocache, 0, 0) < 0)
+		GOTO_ERR(ret, err);
+
+	/* basic tests with zero-copy API's */
+	if (test_mempool_basic(mp_cache, 0, 1) < 0)
 		GOTO_ERR(ret, err);

-	/* basic tests with cache */
-	if (test_mempool_basic(mp_cache, 0) < 0)
+	/* basic tests with user-owned cache and zero-copy API's */
+	if (test_mempool_basic(mp_nocache, 1, 1) < 0)
 		GOTO_ERR(ret, err);

 	/* basic tests with user-owned cache */
-	if (test_mempool_basic(mp_nocache, 1) < 0)
+	if (test_mempool_basic(mp_nocache, 1, 0) < 0)
 		GOTO_ERR(ret, err);

 	/* more basic tests without cache */
@@ -1008,10 +1047,10 @@ test_mempool(void)
 		GOTO_ERR(ret, err);

 	/* test the stack handler */
-	if (test_mempool_basic(mp_stack, 1) < 0)
+	if (test_mempool_basic(mp_stack, 1, 0) < 0)
 		GOTO_ERR(ret, err);

-	if (test_mempool_basic(default_pool, 1) < 0)
+	if (test_mempool_basic(default_pool, 1, 0) < 0)
 		GOTO_ERR(ret, err);

 	/* test mempool event callbacks */
--
2.25.1

[PATCH v2 2/2] test/mempool: add zero-copy API's

[Kamalakshitha Aligeri]

Added mempool test cases with zero-copy get and put API's

Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
Link: https://patchwork.dpdk.org/project/dpdk/patch/20230209145833.129986-1-mb@smartsharesystems.com/

 app/test/test_mempool.c | 81 ++++++++++++++++++++++++++++++-----------
 1 file changed, 60 insertions(+), 21 deletions(-)

diff --git a/app/test/test_mempool.c b/app/test/test_mempool.c
index 8e493eda47..6d29f5bc7b 100644
--- a/app/test/test_mempool.c
+++ b/app/test/test_mempool.c
@@ -74,7 +74,7 @@ my_obj_init(struct rte_mempool *mp, __rte_unused void *arg,

 /* basic tests (done on one core) */
 static int
-test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
+test_mempool_basic(struct rte_mempool *mp, int use_external_cache, int use_zc_api)
 {
 	uint32_t *objnum;
 	void **objtable;
@@ -84,6 +84,7 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 	unsigned i, j;
 	int offset;
 	struct rte_mempool_cache *cache;
+	void **cache_objs;

 	if (use_external_cache) {
 		/* Create a user-owned mempool cache. */
@@ -100,8 +101,13 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 	rte_mempool_dump(stdout, mp);

 	printf("get an object\n");
-	if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
-		GOTO_ERR(ret, out);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj = *cache_objs;
+	} else {
+		if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
+			GOTO_ERR(ret, out);
+	}
 	rte_mempool_dump(stdout, mp);

 	/* tests that improve coverage */
@@ -123,21 +129,41 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 #endif

 	printf("put the object back\n");
-	rte_mempool_generic_put(mp, &obj, 1, cache);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj, sizeof(void *));
+	} else {
+		rte_mempool_generic_put(mp, &obj, 1, cache);
+	}
 	rte_mempool_dump(stdout, mp);

 	printf("get 2 objects\n");
-	if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
-		GOTO_ERR(ret, out);
-	if (rte_mempool_generic_get(mp, &obj2, 1, cache) < 0) {
-		rte_mempool_generic_put(mp, &obj, 1, cache);
-		GOTO_ERR(ret, out);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj = *cache_objs;
+		cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+		obj2 = *cache_objs;
+	} else {
+		if (rte_mempool_generic_get(mp, &obj, 1, cache) < 0)
+			GOTO_ERR(ret, out);
+		if (rte_mempool_generic_get(mp, &obj2, 1, cache) < 0) {
+			rte_mempool_generic_put(mp, &obj, 1, cache);
+			GOTO_ERR(ret, out);
+		}
 	}
 	rte_mempool_dump(stdout, mp);

 	printf("put the objects back\n");
-	rte_mempool_generic_put(mp, &obj, 1, cache);
-	rte_mempool_generic_put(mp, &obj2, 1, cache);
+	if (use_zc_api) {
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj, sizeof(void *));
+		cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+		rte_memcpy(cache_objs, &obj2, sizeof(void *));
+
+	} else {
+		rte_mempool_generic_put(mp, &obj, 1, cache);
+		rte_mempool_generic_put(mp, &obj2, 1, cache);
+	}
 	rte_mempool_dump(stdout, mp);

 	/*
@@ -149,8 +175,13 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 		GOTO_ERR(ret, out);

 	for (i = 0; i < MEMPOOL_SIZE; i++) {
-		if (rte_mempool_generic_get(mp, &objtable[i], 1, cache) < 0)
-			break;
+		if (use_zc_api) {
+			cache_objs = rte_mempool_cache_zc_get_bulk(cache, mp, 1);
+			objtable[i] = *cache_objs;
+		} else {
+			if (rte_mempool_generic_get(mp, &objtable[i], 1, cache) < 0)
+				break;
+		}
 	}

 	/*
@@ -170,8 +201,12 @@ test_mempool_basic(struct rte_mempool *mp, int use_external_cache)
 			if (obj_data[j] != 0)
 				ret = -1;
 		}
-
-		rte_mempool_generic_put(mp, &objtable[i], 1, cache);
+		if (use_zc_api) {
+			cache_objs = rte_mempool_cache_zc_put_bulk(cache, mp, 1);
+			rte_memcpy(cache_objs, &objtable[i], sizeof(void *));
+		} else {
+			rte_mempool_generic_put(mp, &objtable[i], 1, cache);
+		}
 	}

 	free(objtable);
@@ -979,15 +1014,19 @@ test_mempool(void)
 	rte_mempool_list_dump(stdout);

 	/* basic tests without cache */
-	if (test_mempool_basic(mp_nocache, 0) < 0)
+	if (test_mempool_basic(mp_nocache, 0, 0) < 0)
+		GOTO_ERR(ret, err);
+
+	/* basic tests with zero-copy API's */
+	if (test_mempool_basic(mp_cache, 0, 1) < 0)
 		GOTO_ERR(ret, err);

-	/* basic tests with cache */
-	if (test_mempool_basic(mp_cache, 0) < 0)
+	/* basic tests with user-owned cache and zero-copy API's */
+	if (test_mempool_basic(mp_nocache, 1, 1) < 0)
 		GOTO_ERR(ret, err);

 	/* basic tests with user-owned cache */
-	if (test_mempool_basic(mp_nocache, 1) < 0)
+	if (test_mempool_basic(mp_nocache, 1, 0) < 0)
 		GOTO_ERR(ret, err);

 	/* more basic tests without cache */
@@ -1008,10 +1047,10 @@ test_mempool(void)
 		GOTO_ERR(ret, err);

 	/* test the stack handler */
-	if (test_mempool_basic(mp_stack, 1) < 0)
+	if (test_mempool_basic(mp_stack, 1, 0) < 0)
 		GOTO_ERR(ret, err);

-	if (test_mempool_basic(default_pool, 1) < 0)
+	if (test_mempool_basic(default_pool, 1, 0) < 0)
 		GOTO_ERR(ret, err);

 	/* test mempool event callbacks */
--
2.25.1

RE: [PATCH v2 2/2] test/mempool: add zero-copy API's

[Morten Brørup]

> From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> Sent: Friday, 10 February 2023 07.54
> 
> Added mempool test cases with zero-copy get and put API's
> 
> Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>

I already acked v1 of this patch, but here it is again for Patchwork:

Acked-by: Morten Brørup <mb@smartsharesystems.com>

> ---
> Link:
> https://patchwork.dpdk.org/project/dpdk/patch/20230209145833.129986-1-
> mb@smartsharesystems.com/

@David, here's the zero-copy mempool cache API test cases you were asking for.

Re: [PATCH v2 2/2] test/mempool: add zero-copy API's

[Thomas Monjalon]

10/02/2023 08:33, Morten Brørup:
> > From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> > Sent: Friday, 10 February 2023 07.54
> > 
> > Added mempool test cases with zero-copy get and put API's
> > 
> > Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> 
> I already acked v1 of this patch, but here it is again for Patchwork:
> 
> Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 
> > ---
> > Link:
> > https://patchwork.dpdk.org/project/dpdk/patch/20230209145833.129986-1-
> > mb@smartsharesystems.com/
> 
> @David, here's the zero-copy mempool cache API test cases you were asking for.

The unit tests should be squashed in the mempool lib patch.

Also I would to see a review from the mempool maintainers.

RE: [PATCH v2 2/2] test/mempool: add zero-copy API's

[Kamalakshitha Aligeri]

Hi Thomas,

Do you want me to squash the unit tests in the mempool lib patch or do I have to wait for the reviews from mempool maintainers


Thanks,
Kamalakshitha

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Monday, February 20, 2023 5:53 AM
> To: olivier.matz@6wind.com; andrew.rybchenko@oktetlabs.ru; Morten
> Brørup <mb@smartsharesystems.com>
> Cc: Kamalakshitha Aligeri <Kamalakshitha.Aligeri@arm.com>;
> Yuying.Zhang@intel.com; beilei.xing@intel.com;
> bruce.richardson@intel.com; konstantin.ananyev@huawei.com; Honnappa
> Nagarahalli <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; Feifei Wang <Feifei.Wang2@arm.com>;
> david.marchand@redhat.com; dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [PATCH v2 2/2] test/mempool: add zero-copy API's
> 
> 10/02/2023 08:33, Morten Brørup:
> > > From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> > > Sent: Friday, 10 February 2023 07.54
> > >
> > > Added mempool test cases with zero-copy get and put API's
> > >
> > > Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> >
> > I already acked v1 of this patch, but here it is again for Patchwork:
> >
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> >
> > > ---
> > > Link:
> > >
> https://patchwork.dpdk.org/project/dpdk/patch/20230209145833.129986-1-
> > > mb@smartsharesystems.com/
> >
> > @David, here's the zero-copy mempool cache API test cases you were
> asking for.
> 
> The unit tests should be squashed in the mempool lib patch.
> 
> Also I would to see a review from the mempool maintainers.
> 

Re: [PATCH v2 2/2] test/mempool: add zero-copy API's

[Thomas Monjalon]

21/02/2023 21:18, Kamalakshitha Aligeri:
> Hi Thomas,
> 
> Do you want me to squash the unit tests in the mempool lib patch or do I have to wait for the reviews from mempool maintainers

Yes I think you can do the squash if Morten agrees.


> From: Thomas Monjalon <thomas@monjalon.net>
> > 10/02/2023 08:33, Morten Brørup:
> > > > From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> > > > Sent: Friday, 10 February 2023 07.54
> > > >
> > > > Added mempool test cases with zero-copy get and put API's
> > > >
> > > > Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> > >
> > > I already acked v1 of this patch, but here it is again for Patchwork:
> > >
> > > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > >
> > > > ---
> > > > Link:
> > > >
> > https://patchwork.dpdk.org/project/dpdk/patch/20230209145833.129986-1-
> > > > mb@smartsharesystems.com/
> > >
> > > @David, here's the zero-copy mempool cache API test cases you were
> > asking for.
> > 
> > The unit tests should be squashed in the mempool lib patch.
> > 
> > Also I would to see a review from the mempool maintainers.
> > 
> 
> 

RE: [PATCH v2 2/2] test/mempool: add zero-copy API's

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, 22 February 2023 09.01
> 
> 21/02/2023 21:18, Kamalakshitha Aligeri:
> > Hi Thomas,
> >
> > Do you want me to squash the unit tests in the mempool lib patch or do I
> have to wait for the reviews from mempool maintainers
> 
> Yes I think you can do the squash if Morten agrees.

Yes, I agree. And if there are any more changes required before the code is accepted by the maintainers, I will let Kamalakshitha make those changes.


How should the different acks/review tags be handled when squashing two patches into one?

The library patch has:

Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
Acked-by: Chengwen Feng <fengchengwen@huawei.com>
Acked-by: Kamalakshitha Aligeri <Kamalakshitha.aligeri@arm.com>

And the test patch has:

Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
Acked-by: Morten Brørup <mb@smartsharesystems.com>

> 
> 
> > From: Thomas Monjalon <thomas@monjalon.net>
> > > 10/02/2023 08:33, Morten Brørup:
> > > > > From: Kamalakshitha Aligeri [mailto:kamalakshitha.aligeri@arm.com]
> > > > > Sent: Friday, 10 February 2023 07.54
> > > > >
> > > > > Added mempool test cases with zero-copy get and put API's
> > > > >
> > > > > Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > > > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > > > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> > > >
> > > > I already acked v1 of this patch, but here it is again for Patchwork:
> > > >
> > > > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> > > >
> > > > > ---
> > > > > Link:
> > > > >
> > > https://patchwork.dpdk.org/project/dpdk/patch/20230209145833.129986-1-
> > > > > mb@smartsharesystems.com/
> > > >
> > > > @David, here's the zero-copy mempool cache API test cases you were
> > > asking for.
> > >
> > > The unit tests should be squashed in the mempool lib patch.
> > >
> > > Also I would to see a review from the mempool maintainers.

Re: [PATCH v2 2/2] test/mempool: add zero-copy API's

[Thomas Monjalon]

22/02/2023 09:24, Morten Brørup:
> > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > Sent: Wednesday, 22 February 2023 09.01
> > 
> > 21/02/2023 21:18, Kamalakshitha Aligeri:
> > > Hi Thomas,
> > >
> > > Do you want me to squash the unit tests in the mempool lib patch or do I
> > have to wait for the reviews from mempool maintainers
> > 
> > Yes I think you can do the squash if Morten agrees.
> 
> Yes, I agree. And if there are any more changes required before the code is accepted by the maintainers, I will let Kamalakshitha make those changes.
> 
> 
> How should the different acks/review tags be handled when squashing two patches into one?
> 
> The library patch has:
> 
> Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> Acked-by: Kamalakshitha Aligeri <Kamalakshitha.aligeri@arm.com>
> 
> And the test patch has:
> 
> Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> Acked-by: Morten Brørup <mb@smartsharesystems.com>

The cleanest is to remove Reviewed and Acked-by and let reviewer check again.
About the sign-off, you keep both.
About the authorship, you must choose one.

RE: [PATCH v2 2/2] test/mempool: add zero-copy API's

[Morten Brørup]

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Wednesday, 22 February 2023 13.41
> 
> 22/02/2023 09:24, Morten Brørup:
> > > From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > > Sent: Wednesday, 22 February 2023 09.01
> > >
> > > 21/02/2023 21:18, Kamalakshitha Aligeri:
> > > > Hi Thomas,
> > > >
> > > > Do you want me to squash the unit tests in the mempool lib patch
> or do I
> > > have to wait for the reviews from mempool maintainers
> > >
> > > Yes I think you can do the squash if Morten agrees.
> >
> > Yes, I agree. And if there are any more changes required before the
> code is accepted by the maintainers, I will let Kamalakshitha make those
> changes.
> >
> >
> > How should the different acks/review tags be handled when squashing
> two patches into one?
> >
> > The library patch has:
> >
> > Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
> > Acked-by: Konstantin Ananyev <konstantin.v.ananyev@yandex.ru>
> > Acked-by: Chengwen Feng <fengchengwen@huawei.com>
> > Acked-by: Kamalakshitha Aligeri <Kamalakshitha.aligeri@arm.com>
> >
> > And the test patch has:
> >
> > Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> > Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 
> The cleanest is to remove Reviewed and Acked-by and let reviewer check
> again.
> About the sign-off, you keep both.
> About the authorship, you must choose one.

Kamalakshitha, since I designed and fought for the mempool zero-copy API, which I consider the core part of the patch, I would prefer being designated as the author of the squashed patch.

I'm no expert at git, but I think the "From" tag indicates the author. So the tags in the squashed patch should be:

From: Morten Brørup <mb@smartsharesystems.com>
Signed-off-by: Morten Brørup <mb@smartsharesystems.com>
Signed-off-by: Kamalakshitha Aligeri <kamalakshitha.aligeri@arm.com>