[v1,2/6] lib/mbuf: enable parse flags when create mempool

Message ID 20190301080947.91086-3-xiaolong.ye@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series Introduce AF_XDP PMD |

Checks

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

Commit Message

Xiaolong Ye March 1, 2019, 8:09 a.m. UTC
  This give the option that applicaiton can configure each
memory chunk's size precisely. (by MEMPOOL_F_NO_SPREAD).

Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 lib/librte_mbuf/rte_mbuf.c | 15 ++++++++++++---
 lib/librte_mbuf/rte_mbuf.h |  8 +++++++-
 2 files changed, 19 insertions(+), 4 deletions(-)
  

Comments

David Marchand March 5, 2019, 8:30 a.m. UTC | #1
On Fri, Mar 1, 2019 at 9:13 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:

> This give the option that applicaiton can configure each
> memory chunk's size precisely. (by MEMPOOL_F_NO_SPREAD).
>
> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>

Cc: maintainer

---
>  lib/librte_mbuf/rte_mbuf.c | 15 ++++++++++++---
>  lib/librte_mbuf/rte_mbuf.h |  8 +++++++-
>  2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 21f6f7404..0f6fcff28 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -110,7 +110,7 @@ rte_pktmbuf_init(struct rte_mempool *mp,
>  struct rte_mempool *
>  rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
>         unsigned int cache_size, uint16_t priv_size, uint16_t
> data_room_size,
> -       int socket_id, const char *ops_name)
> +       unsigned int flags, int socket_id, const char *ops_name)
>  {
>         struct rte_mempool *mp;
>         struct rte_pktmbuf_pool_private mbp_priv;
>

You can't do that, rte_pktmbuf_pool_create_by_ops is exposed to the user
apps and part of the ABI.
You must define a new internal fonction that takes this flag, keeps the
existing interface and add your new experimental api.
  
Xiaolong Ye March 7, 2019, 3:07 a.m. UTC | #2
Hi David,

Thanks for you comments.

On 03/05, David Marchand wrote:
>On Fri, Mar 1, 2019 at 9:13 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
>
>> This give the option that applicaiton can configure each
>> memory chunk's size precisely. (by MEMPOOL_F_NO_SPREAD).
>>
>> Signed-off-by: Qi Zhang <qi.z.zhang@intel.com>
>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>>
>
>Cc: maintainer
>
>---
>>  lib/librte_mbuf/rte_mbuf.c | 15 ++++++++++++---
>>  lib/librte_mbuf/rte_mbuf.h |  8 +++++++-
>>  2 files changed, 19 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
>> index 21f6f7404..0f6fcff28 100644
>> --- a/lib/librte_mbuf/rte_mbuf.c
>> +++ b/lib/librte_mbuf/rte_mbuf.c
>> @@ -110,7 +110,7 @@ rte_pktmbuf_init(struct rte_mempool *mp,
>>  struct rte_mempool *
>>  rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
>>         unsigned int cache_size, uint16_t priv_size, uint16_t
>> data_room_size,
>> -       int socket_id, const char *ops_name)
>> +       unsigned int flags, int socket_id, const char *ops_name)
>>  {
>>         struct rte_mempool *mp;
>>         struct rte_pktmbuf_pool_private mbp_priv;
>>
>
>You can't do that, rte_pktmbuf_pool_create_by_ops is exposed to the user
>apps and part of the ABI.
>You must define a new internal fonction that takes this flag, keeps the
>existing interface and add your new experimental api.
>

In this case, if I define a new function that takes the flag, it seems would
have a lot of duplicated code with rte_pktmbuf_pool_create_by_ops, do you have
any suggestions for better handling?

Thanks,
Xiaolong

>
>-- 
>David Marchand
  

Patch

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 21f6f7404..0f6fcff28 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -110,7 +110,7 @@  rte_pktmbuf_init(struct rte_mempool *mp,
 struct rte_mempool *
 rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
 	unsigned int cache_size, uint16_t priv_size, uint16_t data_room_size,
-	int socket_id, const char *ops_name)
+	unsigned int flags, int socket_id, const char *ops_name)
 {
 	struct rte_mempool *mp;
 	struct rte_pktmbuf_pool_private mbp_priv;
@@ -130,7 +130,7 @@  rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
 	mbp_priv.mbuf_priv_size = priv_size;
 
 	mp = rte_mempool_create_empty(name, n, elt_size, cache_size,
-		 sizeof(struct rte_pktmbuf_pool_private), socket_id, 0);
+		 sizeof(struct rte_pktmbuf_pool_private), socket_id, flags);
 	if (mp == NULL)
 		return NULL;
 
@@ -164,9 +164,18 @@  rte_pktmbuf_pool_create(const char *name, unsigned int n,
 	int socket_id)
 {
 	return rte_pktmbuf_pool_create_by_ops(name, n, cache_size, priv_size,
-			data_room_size, socket_id, NULL);
+			data_room_size, 0, socket_id, NULL);
 }
 
+/* helper to create a mbuf pool with NO_SPREAD */
+struct rte_mempool *
+rte_pktmbuf_pool_create_with_flags(const char *name, unsigned int n,
+	unsigned int cache_size, uint16_t priv_size, uint16_t data_room_size,
+	unsigned int flags, int socket_id)
+{
+	return rte_pktmbuf_pool_create_by_ops(name, n, cache_size, priv_size,
+			data_room_size, flags, socket_id, NULL);
+}
 /* do some sanity checks on a mbuf: panic if it fails */
 void
 rte_mbuf_sanity_check(const struct rte_mbuf *m, int is_header)
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index d961ccaf6..7a3faf11c 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -1266,6 +1266,12 @@  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 *
+rte_pktmbuf_pool_create_with_flags(const char *name, unsigned int n,
+	unsigned int cache_size, uint16_t priv_size, uint16_t data_room_size,
+	unsigned int flags, int socket_id);
+
+
 /**
  * Create a mbuf pool with a given mempool ops name
  *
@@ -1306,7 +1312,7 @@  rte_pktmbuf_pool_create(const char *name, unsigned n,
 struct rte_mempool *
 rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
 	unsigned int cache_size, uint16_t priv_size, uint16_t data_room_size,
-	int socket_id, const char *ops_name);
+	unsigned int flags, int socket_id, const char *ops_name);
 
 /**
  * Get the data room size of mbufs stored in a pktmbuf_pool