[v4,4/5] app/testpmd: add mempool with external data buffers

Message ID 1579179869-32508-5-git-send-email-viacheslavo@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series mbuf: detach mbuf with pinned external buffer |

Checks

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

Commit Message

Slava Ovsiienko Jan. 16, 2020, 1:04 p.m. UTC
  The new mbuf pool type is added to testpmd. To engage the
mbuf pool with externally attached data buffers the parameter
"--mp-alloc=xbuf" should be specified in testpmd command line.

The objective of this patch is just to test whether mbuf pool
with externally attached data buffers works OK. The memory for
data buffers is allocated from DPDK memory, so this is not
"true" external memory from some physical device (this is
supposed the most common use case for such kind of mbuf pool).

The user should be aware that not all drivers support the mbuf
with EXT_ATTACHED_BUF flags set in newly allocated mbuf (many
PMDs just overwrite ol_flags field and flag value is getting
lost).

Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
---
 app/test-pmd/config.c     |  2 ++
 app/test-pmd/flowgen.c    |  3 +-
 app/test-pmd/parameters.c |  2 ++
 app/test-pmd/testpmd.c    | 81 +++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.h    |  4 ++-
 app/test-pmd/txonly.c     |  3 +-
 6 files changed, 92 insertions(+), 3 deletions(-)
  

Comments

Olivier Matz Jan. 20, 2020, 2:11 p.m. UTC | #1
On Thu, Jan 16, 2020 at 01:04:28PM +0000, Viacheslav Ovsiienko wrote:
> The new mbuf pool type is added to testpmd. To engage the
> mbuf pool with externally attached data buffers the parameter
> "--mp-alloc=xbuf" should be specified in testpmd command line.
> 
> The objective of this patch is just to test whether mbuf pool
> with externally attached data buffers works OK. The memory for
> data buffers is allocated from DPDK memory, so this is not
> "true" external memory from some physical device (this is
> supposed the most common use case for such kind of mbuf pool).
> 
> The user should be aware that not all drivers support the mbuf
> with EXT_ATTACHED_BUF flags set in newly allocated mbuf (many
> PMDs just overwrite ol_flags field and flag value is getting
> lost).
> 
> Signed-off-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>
> ---
>  app/test-pmd/config.c     |  2 ++
>  app/test-pmd/flowgen.c    |  3 +-
>  app/test-pmd/parameters.c |  2 ++
>  app/test-pmd/testpmd.c    | 81 +++++++++++++++++++++++++++++++++++++++++++++++
>  app/test-pmd/testpmd.h    |  4 ++-
>  app/test-pmd/txonly.c     |  3 +-
>  6 files changed, 92 insertions(+), 3 deletions(-)
> 
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 9da1ffb..5c6fe18 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -2395,6 +2395,8 @@ struct igb_ring_desc_16_bytes {
>  		return "xmem";
>  	case MP_ALLOC_XMEM_HUGE:
>  		return "xmemhuge";
> +	case MP_ALLOC_XBUF:
> +		return "xbuf";
>  	default:
>  		return "invalid";
>  	}
> diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c
> index 03b72aa..ae50cdc 100644
> --- a/app/test-pmd/flowgen.c
> +++ b/app/test-pmd/flowgen.c
> @@ -199,7 +199,8 @@
>  							   sizeof(*ip_hdr));
>  		pkt->nb_segs		= 1;
>  		pkt->pkt_len		= pkt_size;
> -		pkt->ol_flags		= ol_flags;
> +		pkt->ol_flags		&= EXT_ATTACHED_MBUF;
> +		pkt->ol_flags		|= ol_flags;
>  		pkt->vlan_tci		= vlan_tci;
>  		pkt->vlan_tci_outer	= vlan_tci_outer;
>  		pkt->l2_len		= sizeof(struct rte_ether_hdr);

This shows that we have to be careful when using a mempool with
external memory pinned mbufs. Maybe that's something that should
be mentionned in the release note?

That's not the first time I'm asking myself if ol_flags shouldn't be
splitted in ol_flags and flags. Certainly something to think about for
next ABI breakage release.

Acked-by: Olivier Matz <olivier.matz@6wind.com>
  

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 9da1ffb..5c6fe18 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -2395,6 +2395,8 @@  struct igb_ring_desc_16_bytes {
 		return "xmem";
 	case MP_ALLOC_XMEM_HUGE:
 		return "xmemhuge";
+	case MP_ALLOC_XBUF:
+		return "xbuf";
 	default:
 		return "invalid";
 	}
diff --git a/app/test-pmd/flowgen.c b/app/test-pmd/flowgen.c
index 03b72aa..ae50cdc 100644
--- a/app/test-pmd/flowgen.c
+++ b/app/test-pmd/flowgen.c
@@ -199,7 +199,8 @@ 
 							   sizeof(*ip_hdr));
 		pkt->nb_segs		= 1;
 		pkt->pkt_len		= pkt_size;
-		pkt->ol_flags		= ol_flags;
+		pkt->ol_flags		&= EXT_ATTACHED_MBUF;
+		pkt->ol_flags		|= ol_flags;
 		pkt->vlan_tci		= vlan_tci;
 		pkt->vlan_tci_outer	= vlan_tci_outer;
 		pkt->l2_len		= sizeof(struct rte_ether_hdr);
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 2e7a504..6340104 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -841,6 +841,8 @@ 
 					mp_alloc_type = MP_ALLOC_XMEM;
 				else if (!strcmp(optarg, "xmemhuge"))
 					mp_alloc_type = MP_ALLOC_XMEM_HUGE;
+				else if (!strcmp(optarg, "xbuf"))
+					mp_alloc_type = MP_ALLOC_XBUF;
 				else
 					rte_exit(EXIT_FAILURE,
 						"mp-alloc %s invalid - must be: "
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 2eec8af..5f910ba 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -78,6 +78,7 @@ 
 #endif
 
 #define EXTMEM_HEAP_NAME "extmem"
+#define EXTBUF_ZONE_SIZE RTE_PGSIZE_2M
 
 uint16_t verbose_level = 0; /**< Silent by default. */
 int testpmd_logtype; /**< Log type for testpmd logs */
@@ -865,6 +866,66 @@  struct extmem_param {
 	}
 }
 
+static unsigned int
+setup_extbuf(uint32_t nb_mbufs, uint16_t mbuf_sz, unsigned int socket_id,
+	    char *pool_name, struct rte_pktmbuf_extmem **ext_mem)
+{
+	struct rte_pktmbuf_extmem *xmem;
+	unsigned int ext_num, zone_num, elt_num;
+	uint16_t elt_size;
+
+	elt_size = RTE_ALIGN_CEIL(mbuf_sz, RTE_CACHE_LINE_SIZE);
+	elt_num = EXTBUF_ZONE_SIZE / elt_size;
+	zone_num = (nb_mbufs + elt_num - 1) / elt_num;
+
+	xmem = malloc(sizeof(struct rte_pktmbuf_extmem) * zone_num);
+	if (xmem == NULL) {
+		TESTPMD_LOG(ERR, "Cannot allocate memory for "
+				 "external buffer descriptors\n");
+		*ext_mem = NULL;
+		return 0;
+	}
+	for (ext_num = 0; ext_num < zone_num; ext_num++) {
+		struct rte_pktmbuf_extmem *xseg = xmem + ext_num;
+		const struct rte_memzone *mz;
+		char mz_name[RTE_MEMZONE_NAMESIZE];
+		int ret;
+
+		ret = snprintf(mz_name, sizeof(mz_name),
+			RTE_MEMPOOL_MZ_FORMAT "_xb_%u", pool_name, ext_num);
+		if (ret < 0 || ret >= (int)sizeof(mz_name)) {
+			errno = ENAMETOOLONG;
+			ext_num = 0;
+			break;
+		}
+		mz = rte_memzone_reserve_aligned(mz_name, EXTBUF_ZONE_SIZE,
+						 socket_id,
+						 RTE_MEMZONE_IOVA_CONTIG |
+						 RTE_MEMZONE_1GB |
+						 RTE_MEMZONE_SIZE_HINT_ONLY,
+						 EXTBUF_ZONE_SIZE);
+		if (mz == NULL) {
+			/*
+			 * The caller exits on external buffer creation
+			 * error, so there is no need to free memzones.
+			 */
+			errno = ENOMEM;
+			ext_num = 0;
+			break;
+		}
+		xseg->buf_ptr = mz->addr;
+		xseg->buf_iova = mz->iova;
+		xseg->buf_len = EXTBUF_ZONE_SIZE;
+		xseg->elt_size = elt_size;
+	}
+	if (ext_num == 0 && xmem != NULL) {
+		free(xmem);
+		xmem = NULL;
+	}
+	*ext_mem = xmem;
+	return ext_num;
+}
+
 /*
  * Configuration initialisation done once at init time.
  */
@@ -933,6 +994,26 @@  struct extmem_param {
 					heap_socket);
 			break;
 		}
+	case MP_ALLOC_XBUF:
+		{
+			struct rte_pktmbuf_extmem *ext_mem;
+			unsigned int ext_num;
+
+			ext_num = setup_extbuf(nb_mbuf,	mbuf_seg_size,
+					       socket_id, pool_name, &ext_mem);
+			if (ext_num == 0)
+				rte_exit(EXIT_FAILURE,
+					 "Can't create pinned data buffers\n");
+
+			TESTPMD_LOG(INFO, "preferred mempool ops selected: %s\n",
+					rte_mbuf_best_mempool_ops());
+			rte_mp = rte_pktmbuf_pool_create_extbuf
+					(pool_name, nb_mbuf, mb_mempool_cache,
+					 0, mbuf_seg_size, socket_id,
+					 ext_mem, ext_num);
+			free(ext_mem);
+			break;
+		}
 	default:
 		{
 			rte_exit(EXIT_FAILURE, "Invalid mempool creation mode\n");
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 857a11f..a47f214 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -76,8 +76,10 @@  enum {
 	/**< allocate mempool natively, but populate using anonymous memory */
 	MP_ALLOC_XMEM,
 	/**< allocate and populate mempool using anonymous memory */
-	MP_ALLOC_XMEM_HUGE
+	MP_ALLOC_XMEM_HUGE,
 	/**< allocate and populate mempool using anonymous hugepage memory */
+	MP_ALLOC_XBUF
+	/**< allocate mempool natively, use rte_pktmbuf_pool_create_extbuf */
 };
 
 #ifdef RTE_TEST_PMD_RECORD_BURST_STATS
diff --git a/app/test-pmd/txonly.c b/app/test-pmd/txonly.c
index 3caf281..871cf6c 100644
--- a/app/test-pmd/txonly.c
+++ b/app/test-pmd/txonly.c
@@ -170,7 +170,8 @@ 
 
 	rte_pktmbuf_reset_headroom(pkt);
 	pkt->data_len = tx_pkt_seg_lengths[0];
-	pkt->ol_flags = ol_flags;
+	pkt->ol_flags &= EXT_ATTACHED_MBUF;
+	pkt->ol_flags |= ol_flags;
 	pkt->vlan_tci = vlan_tci;
 	pkt->vlan_tci_outer = vlan_tci_outer;
 	pkt->l2_len = sizeof(struct rte_ether_hdr);