[dpdk-dev,v7,5/5] mbuf: allow apps to change default mempool ops

Message ID 1464874043-67467-6-git-send-email-david.hunt@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Hunt, David June 2, 2016, 1:27 p.m. UTC
  By default, the mempool ops used for mbuf allocations is a multi
producer and multi consumer ring. We could imagine a target (maybe some
network processors?) that provides an hardware-assisted pool
mechanism. In this case, the default configuration for this architecture
would contain a different value for RTE_MBUF_DEFAULT_MEMPOOL_OPS.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Signed-off-by: David Hunt <david.hunt@intel.com>
---
 config/common_base         |  1 +
 lib/librte_mbuf/rte_mbuf.c | 26 ++++++++++++++++++++++----
 2 files changed, 23 insertions(+), 4 deletions(-)
  

Comments

Olivier Matz June 3, 2016, 12:28 p.m. UTC | #1
> [PATCH v7 5/5] mbuf: allow apps to change default mempool ops

Should the title be fixed?
I don't feel this allows application to change the default ops.
  
Hunt, David June 3, 2016, 2:06 p.m. UTC | #2
On 6/3/2016 1:28 PM, Olivier MATZ wrote:
>> [PATCH v7 5/5] mbuf: allow apps to change default mempool ops
> Should the title be fixed?
> I don't feel this allows application to change the default ops.

Allow _user_ to change default mempool ops, I think.
  
Olivier Matz June 3, 2016, 2:10 p.m. UTC | #3
On 06/03/2016 04:06 PM, Hunt, David wrote:
> 
> 
> On 6/3/2016 1:28 PM, Olivier MATZ wrote:
>>> [PATCH v7 5/5] mbuf: allow apps to change default mempool ops
>> Should the title be fixed?
>> I don't feel this allows application to change the default ops.
> 
> Allow _user_ to change default mempool ops, I think.

make default mempool ops configurable at build
 ?
  
Hunt, David June 3, 2016, 2:14 p.m. UTC | #4
On 6/3/2016 3:10 PM, Olivier Matz wrote:
>
> On 06/03/2016 04:06 PM, Hunt, David wrote:
>>
>> On 6/3/2016 1:28 PM, Olivier MATZ wrote:
>>>> [PATCH v7 5/5] mbuf: allow apps to change default mempool ops
>>> Should the title be fixed?
>>> I don't feel this allows application to change the default ops.
>> Allow _user_ to change default mempool ops, I think.
> make default mempool ops configurable at build
>   ?

Yup :)
  

Patch

diff --git a/config/common_base b/config/common_base
index 47c26f6..899c038 100644
--- a/config/common_base
+++ b/config/common_base
@@ -394,6 +394,7 @@  CONFIG_RTE_LIBRTE_MEMPOOL_DEBUG=n
 #
 CONFIG_RTE_LIBRTE_MBUF=y
 CONFIG_RTE_LIBRTE_MBUF_DEBUG=n
+CONFIG_RTE_MBUF_DEFAULT_MEMPOOL_OPS="ring_mp_mc"
 CONFIG_RTE_MBUF_REFCNT_ATOMIC=y
 CONFIG_RTE_PKTMBUF_HEADROOM=128
 
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index eec1456..491230c 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -153,6 +153,7 @@  rte_pktmbuf_pool_create(const char *name, unsigned n,
 	unsigned cache_size, uint16_t priv_size, uint16_t data_room_size,
 	int socket_id)
 {
+	struct rte_mempool *mp;
 	struct rte_pktmbuf_pool_private mbp_priv;
 	unsigned elt_size;
 
@@ -167,10 +168,27 @@  rte_pktmbuf_pool_create(const char *name, unsigned n,
 	mbp_priv.mbuf_data_room_size = data_room_size;
 	mbp_priv.mbuf_priv_size = priv_size;
 
-	return rte_mempool_create(name, n, elt_size,
-		cache_size, sizeof(struct rte_pktmbuf_pool_private),
-		rte_pktmbuf_pool_init, &mbp_priv, rte_pktmbuf_init, NULL,
-		socket_id, 0);
+	mp = rte_mempool_create_empty(name, n, elt_size, cache_size,
+		 sizeof(struct rte_pktmbuf_pool_private), socket_id, 0);
+	if (mp == NULL)
+		return NULL;
+
+	rte_errno = rte_mempool_set_ops_byname(mp,
+			RTE_MBUF_DEFAULT_MEMPOOL_OPS);
+	if (rte_errno != 0) {
+		RTE_LOG(ERR, MBUF, "error setting mempool handler\n");
+		return NULL;
+	}
+	rte_pktmbuf_pool_init(mp, &mbp_priv);
+
+	if (rte_mempool_populate_default(mp) < 0) {
+		rte_mempool_free(mp);
+		return NULL;
+	}
+
+	rte_mempool_obj_iter(mp, rte_pktmbuf_init, NULL);
+
+	return mp;
 }
 
 /* do some sanity checks on a mbuf: panic if it fails */