diff mbox series

[v2] mbuf: extend pktmbuf pool private structure

Message ID 20191121122810.147351-1-shahafs@mellanox.com (mailing list archive)
State Superseded, archived
Headers
Series [v2] mbuf: extend pktmbuf pool private structure |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation fail Compilation issues
ci/travis-robot success Travis build: passed

Commit Message

Shahaf Shuler Nov. 21, 2019, 12:28 p.m. UTC 
  With the API and ABI freeze ahead, it will be good to reserve
some bits on the private structure for future use.

Otherwise we will potentially need to maintain two different
private structure during 2020 period.

There is already one use case for those reserved bits[1]

The reserved field should be set to 0 by the user.

[1]
https://patches.dpdk.org/patch/63077/

Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
---
On v2:
 - rename new field to flags
 - add extra validation in code that flags = 0

---
 lib/librte_mbuf/rte_mbuf.c | 10 +++++++---
 lib/librte_mbuf/rte_mbuf.h |  1 +
 2 files changed, 8 insertions(+), 3 deletions(-)

Comments

Olivier Matz Nov. 21, 2019, 2:31 p.m. UTC | #1 
On Thu, Nov 21, 2019 at 12:28:18PM +0000, Shahaf Shuler wrote:
> With the API and ABI freeze ahead, it will be good to reserve
> some bits on the private structure for future use.
> 
> Otherwise we will potentially need to maintain two different
> private structure during 2020 period.
> 
> There is already one use case for those reserved bits[1]
> 
> The reserved field should be set to 0 by the user.
> 
> [1]
> https://patches.dpdk.org/patch/63077/
> 
> Signed-off-by: Shahaf Shuler <shahafs@mellanox.com>
> ---
> On v2:
>  - rename new field to flags
>  - add extra validation in code that flags = 0
> 
> ---
>  lib/librte_mbuf/rte_mbuf.c | 10 +++++++---
>  lib/librte_mbuf/rte_mbuf.h |  1 +
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
> index 35df1c4c38..bd64c55fe6 100644
> --- a/lib/librte_mbuf/rte_mbuf.c
> +++ b/lib/librte_mbuf/rte_mbuf.c
> @@ -50,6 +50,7 @@ rte_pktmbuf_pool_init(struct rte_mempool *mp, void *opaque_arg)
>  	user_mbp_priv = opaque_arg;
>  	if (user_mbp_priv == NULL) {
>  		default_mbp_priv.mbuf_priv_size = 0;
> +		default_mbp_priv.flags = 0;
>  		if (mp->elt_size > sizeof(struct rte_mbuf))
>  			roomsz = mp->elt_size - sizeof(struct rte_mbuf);
>  		else
> @@ -61,6 +62,7 @@ rte_pktmbuf_pool_init(struct rte_mempool *mp, void *opaque_arg)
>  	RTE_ASSERT(mp->elt_size >= sizeof(struct rte_mbuf) +
>  		user_mbp_priv->mbuf_data_room_size +
>  		user_mbp_priv->mbuf_priv_size);
> +	RTE_ASSERT(user_mbp_priv->flags == 0);
>  
>  	mbp_priv = rte_mempool_get_priv(mp);
>  	memcpy(mbp_priv, user_mbp_priv, sizeof(*mbp_priv));
> @@ -113,7 +115,11 @@ rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
>  	int socket_id, const char *ops_name)
>  {
>  	struct rte_mempool *mp;
> -	struct rte_pktmbuf_pool_private mbp_priv;
> +	struct rte_pktmbuf_pool_private mbp_priv = {
> +		.mbuf_data_room_size = data_room_size,
> +		.mbuf_priv_size = priv_size,
> +		.flags = 0,
> +	};
>  	const char *mp_ops_name = ops_name;
>  	unsigned elt_size;
>  	int ret;
> @@ -126,8 +132,6 @@ rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
>  	}
>  	elt_size = sizeof(struct rte_mbuf) + (unsigned)priv_size +
>  		(unsigned)data_room_size;
> -	mbp_priv.mbuf_data_room_size = data_room_size;
> -	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);
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 92d81972ab..219b110b76 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -303,6 +303,7 @@ rte_mbuf_to_priv(struct rte_mbuf *m)
>  struct rte_pktmbuf_pool_private {
>  	uint16_t mbuf_data_room_size; /**< Size of data space in each mbuf. */
>  	uint16_t mbuf_priv_size;      /**< Size of private area in each mbuf. */
> +	uint32_t flags; /**< reserved for future use. */
>  };
>  
>  #ifdef RTE_LIBRTE_MBUF_DEBUG
> -- 
> 2.12.0
> 

Sorry I missed the last version in my previous mail.

I think in examples/ntb/ntb_fwd.c:ntb_mbuf_pool_create() should be
updated too. To me, it looks slightly better to not name "flags"
explicitly (ie. use a memset instead), to avoid doing the same
change again in the future.

Thanks,
Olivier
Shahaf Shuler Nov. 24, 2019, 5:52 a.m. UTC | #2 
Thursday, November 21, 2019 4:32 PM, Olivier Matz:
> Subject: Re: [PATCH v2] mbuf: extend pktmbuf pool private structure
> 
> On Thu, Nov 21, 2019 at 12:28:18PM +0000, Shahaf Shuler wrote:

[...[

> >
> 
> Sorry I missed the last version in my previous mail.
> 
> I think in examples/ntb/ntb_fwd.c:ntb_mbuf_pool_create() should be
> updated too. To me, it looks slightly better to not name "flags"
> explicitly (ie. use a memset instead), to avoid doing the same
> change again in the future.

Done,
Checkout the v3. 

> 
> Thanks,
> Olivier
diff mbox series

Patch

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 35df1c4c38..bd64c55fe6 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -50,6 +50,7 @@  rte_pktmbuf_pool_init(struct rte_mempool *mp, void *opaque_arg)
 	user_mbp_priv = opaque_arg;
 	if (user_mbp_priv == NULL) {
 		default_mbp_priv.mbuf_priv_size = 0;
+		default_mbp_priv.flags = 0;
 		if (mp->elt_size > sizeof(struct rte_mbuf))
 			roomsz = mp->elt_size - sizeof(struct rte_mbuf);
 		else
@@ -61,6 +62,7 @@  rte_pktmbuf_pool_init(struct rte_mempool *mp, void *opaque_arg)
 	RTE_ASSERT(mp->elt_size >= sizeof(struct rte_mbuf) +
 		user_mbp_priv->mbuf_data_room_size +
 		user_mbp_priv->mbuf_priv_size);
+	RTE_ASSERT(user_mbp_priv->flags == 0);
 
 	mbp_priv = rte_mempool_get_priv(mp);
 	memcpy(mbp_priv, user_mbp_priv, sizeof(*mbp_priv));
@@ -113,7 +115,11 @@  rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
 	int socket_id, const char *ops_name)
 {
 	struct rte_mempool *mp;
-	struct rte_pktmbuf_pool_private mbp_priv;
+	struct rte_pktmbuf_pool_private mbp_priv = {
+		.mbuf_data_room_size = data_room_size,
+		.mbuf_priv_size = priv_size,
+		.flags = 0,
+	};
 	const char *mp_ops_name = ops_name;
 	unsigned elt_size;
 	int ret;
@@ -126,8 +132,6 @@  rte_pktmbuf_pool_create_by_ops(const char *name, unsigned int n,
 	}
 	elt_size = sizeof(struct rte_mbuf) + (unsigned)priv_size +
 		(unsigned)data_room_size;
-	mbp_priv.mbuf_data_room_size = data_room_size;
-	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);
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 92d81972ab..219b110b76 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -303,6 +303,7 @@  rte_mbuf_to_priv(struct rte_mbuf *m)
 struct rte_pktmbuf_pool_private {
 	uint16_t mbuf_data_room_size; /**< Size of data space in each mbuf. */
 	uint16_t mbuf_priv_size;      /**< Size of private area in each mbuf. */
+	uint32_t flags; /**< reserved for future use. */
 };
 
 #ifdef RTE_LIBRTE_MBUF_DEBUG