From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 10065A04C1; Thu, 21 Nov 2019 15:31:57 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 275402BA3; Thu, 21 Nov 2019 15:31:56 +0100 (CET) Received: from proxy.6wind.com (host.76.145.23.62.rev.coltfrance.com [62.23.145.76]) by dpdk.org (Postfix) with ESMTP id 937A02BA2 for ; Thu, 21 Nov 2019 15:31:54 +0100 (CET) Received: from glumotte.dev.6wind.com (unknown [10.16.0.195]) by proxy.6wind.com (Postfix) with ESMTP id 7292B344444; Thu, 21 Nov 2019 15:31:54 +0100 (CET) Date: Thu, 21 Nov 2019 15:31:54 +0100 From: Olivier Matz To: Shahaf Shuler Cc: Thomas Monjalon , "dev@dpdk.org" Message-ID: <20191121143154.GE14387@glumotte.dev.6wind.com> References: <20191118100247.74241-1-shahafs@mellanox.com> <20191121122810.147351-1-shahafs@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20191121122810.147351-1-shahafs@mellanox.com> User-Agent: Mutt/1.10.1 (2018-07-13) Subject: Re: [dpdk-dev] [PATCH v2] mbuf: extend pktmbuf pool private structure X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 > --- > 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