From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f53.google.com (mail-wm0-f53.google.com [74.125.82.53]) by dpdk.org (Postfix) with ESMTP id 8EB04235 for ; Mon, 3 Jul 2017 18:37:11 +0200 (CEST) Received: by mail-wm0-f53.google.com with SMTP id i127so114253830wma.0 for ; Mon, 03 Jul 2017 09:37:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=4oW5ttkHvAy4/p4sFY8+qb+0oBwceOpz52dRtt1Wc0c=; b=QLLW0pUO6yUb2leUA2yiJEutIgxAEzGz5vKdl0E4dn49pwdI7muTQmBkmoau6zAmxO wuTT1dnvzVFwdbYRwtmMvVWd364OIDwgdhv9F2xPGABnkBl/IZ8C5XNxtKcTzXjgELPL cnZtJalbPdA62MO7sZbah2p0FQPiS4w8SqJ5iqlfxjRW456GFHfF2pDvoD85j5hKIhtp p0KqNeSvyUnIoPIfWyK3+NTvzdTgkEHy9LXIz7xCGjqApUYMt71vuJL8NlcFrT8zcs3k CnOV9So3dXVE/guCZW6mOBOo80fz3/N0ruJ9mT9WHHs6MW+mFbUq7A68W5JvQfhDarqU iuHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=4oW5ttkHvAy4/p4sFY8+qb+0oBwceOpz52dRtt1Wc0c=; b=luZo56up8OPG8r4M7jRRBx4iKDuj4KNNZiPfoY7gHUA1uLrw3uloBHr9tenHL1IWSj zZzz46zMXEuiw+i3cNYWrMVxwHLGPsFTJh7gnibjEgb3DnT0VCfddwES5tRrSCgJtyNx 6402US6A2QNFc4Gm+MRgy6IGallnq9AuSwn/CYHXR89NvrRTiJYZwkGrxIWLRDDh403E mMARUP6eQ1BrQtPwd9C/oRGAMTMaJ6JpSkO8h4dqI1MCtIE4IIgCbl5bl288tkQdenjo VAu9vbnXKMyLCASVz+Fd/lZgHzuAZzGsHTG1iSYKEQMw6125SyHIIhfS8rBbCDrS+M3N GoGA== X-Gm-Message-State: AIVw110Ivd7EJgw7THQF+rRCLMx0j2ZnDub8NErwJF4F1okh6H3WZQ/5 cMuw9IvT6f0nV2yjtuI= X-Received: by 10.28.21.80 with SMTP id 77mr4574268wmv.79.1499099831116; Mon, 03 Jul 2017 09:37:11 -0700 (PDT) Received: from platinum (2a01cb0c03c651000226b0fffeed02fc.ipv6.abo.wanadoo.fr. [2a01:cb0c:3c6:5100:226:b0ff:feed:2fc]) by smtp.gmail.com with ESMTPSA id h6sm4819055wmf.31.2017.07.03.09.37.10 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 03 Jul 2017 09:37:11 -0700 (PDT) Date: Mon, 3 Jul 2017 18:37:05 +0200 From: Olivier Matz To: Santosh Shukla Cc: dev@dpdk.org, thomas@monjalon.net, hemant.agrawal@nxp.com, jerin.jacob@caviumnetworks.com, bruce.richardson@intel.com Message-ID: <20170703183705.627d185a@platinum> In-Reply-To: <20170621173248.1313-2-santosh.shukla@caviumnetworks.com> References: <20170621173248.1313-1-santosh.shukla@caviumnetworks.com> <20170621173248.1313-2-santosh.shukla@caviumnetworks.com> X-Mailer: Claws Mail 3.14.1 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH 1/4] mempool: get the external mempool capability 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: , X-List-Received-Date: Mon, 03 Jul 2017 16:37:11 -0000 Hi Santosh, On Wed, 21 Jun 2017 17:32:45 +0000, Santosh Shukla wrote: > Allow external mempool to advertise its capability. > A handler been introduced called rte_mempool_ops_get_hw_cap. > - Upon ->get_hw_cap call, mempool driver will advertise > capability by returning flag. > - Common layer updates flag value in 'mp->flags'. > > Signed-off-by: Santosh Shukla > Signed-off-by: Jerin Jacob I guess you've already seen the compilation issue when shared libs are enabled: http://dpdk.org/dev/patchwork/patch/25603 > --- > lib/librte_mempool/rte_mempool.c | 5 +++++ > lib/librte_mempool/rte_mempool.h | 20 ++++++++++++++++++++ > lib/librte_mempool/rte_mempool_ops.c | 14 ++++++++++++++ > lib/librte_mempool/rte_mempool_version.map | 7 +++++++ > 4 files changed, 46 insertions(+) > > diff --git a/lib/librte_mempool/rte_mempool.c b/lib/librte_mempool/rte_mempool.c > index f65310f60..045baef45 100644 > --- a/lib/librte_mempool/rte_mempool.c > +++ b/lib/librte_mempool/rte_mempool.c > @@ -527,6 +527,11 @@ rte_mempool_populate_default(struct rte_mempool *mp) > if (mp->nb_mem_chunks != 0) > return -EEXIST; > > + /* Get external mempool capability */ > + ret = rte_mempool_ops_get_hw_cap(mp); "hw" can be removed since some handlers are software (the other occurences of hw should be removed too) "capabilities" is clearer than "cap" So I suggest rte_mempool_ops_get_capabilities() instead With this name, the comment above becomes overkill... > + if (ret != -ENOENT) -ENOTSUP looks more appropriate (like in ethdev) > + mp->flags |= ret; I'm wondering if these capability flags should be mixed with other mempool flags. We can maybe remove this code above and directly call rte_mempool_ops_get_capabilities() when we need to get them. > + > if (rte_xen_dom0_supported()) { > pg_sz = RTE_PGSIZE_2M; > pg_shift = rte_bsf32(pg_sz); > diff --git a/lib/librte_mempool/rte_mempool.h b/lib/librte_mempool/rte_mempool.h > index a65f1a79d..c3cdc77e4 100644 > --- a/lib/librte_mempool/rte_mempool.h > +++ b/lib/librte_mempool/rte_mempool.h > @@ -390,6 +390,12 @@ typedef int (*rte_mempool_dequeue_t)(struct rte_mempool *mp, > */ > typedef unsigned (*rte_mempool_get_count)(const struct rte_mempool *mp); > > +/** > + * Get the mempool hw capability. > + */ > +typedef int (*rte_mempool_get_hw_cap_t)(struct rte_mempool *mp); > + > + If possible, use "const struct rte_mempool *mp" Since flags are unsigned, I would also prefer a function returning an int (0 on success, negative on error) and writing to an unsigned pointer provided by the user. > /** Structure defining mempool operations structure */ > struct rte_mempool_ops { > char name[RTE_MEMPOOL_OPS_NAMESIZE]; /**< Name of mempool ops struct. */ > @@ -398,6 +404,7 @@ struct rte_mempool_ops { > rte_mempool_enqueue_t enqueue; /**< Enqueue an object. */ > rte_mempool_dequeue_t dequeue; /**< Dequeue an object. */ > rte_mempool_get_count get_count; /**< Get qty of available objs. */ > + rte_mempool_get_hw_cap_t get_hw_cap; /**< Get hw capability */ > } __rte_cache_aligned; > > #define RTE_MEMPOOL_MAX_OPS_IDX 16 /**< Max registered ops structs */ > @@ -509,6 +516,19 @@ rte_mempool_ops_enqueue_bulk(struct rte_mempool *mp, void * const *obj_table, > unsigned > rte_mempool_ops_get_count(const struct rte_mempool *mp); > > + > +/** > + * @internal wrapper for mempool_ops get_hw_cap callback. > + * > + * @param mp > + * Pointer to the memory pool. > + * @return > + * - On success; Valid capability flag. > + * - On failure; -ENOENT error code i.e. implementation not supported. The possible values for the capability flags should be better described. > + */ > +int > +rte_mempool_ops_get_hw_cap(struct rte_mempool *mp); > + > /** > * @internal wrapper for mempool_ops free callback. > * > diff --git a/lib/librte_mempool/rte_mempool_ops.c b/lib/librte_mempool/rte_mempool_ops.c > index 5f24de250..3a09f5d32 100644 > --- a/lib/librte_mempool/rte_mempool_ops.c > +++ b/lib/librte_mempool/rte_mempool_ops.c > @@ -85,6 +85,7 @@ rte_mempool_register_ops(const struct rte_mempool_ops *h) > ops->enqueue = h->enqueue; > ops->dequeue = h->dequeue; > ops->get_count = h->get_count; > + ops->get_hw_cap = h->get_hw_cap; > > rte_spinlock_unlock(&rte_mempool_ops_table.sl); > > @@ -123,6 +124,19 @@ rte_mempool_ops_get_count(const struct rte_mempool *mp) > return ops->get_count(mp); > } > > +/* wrapper to get external mempool capability. */ > +int > +rte_mempool_ops_get_hw_cap(struct rte_mempool *mp) > +{ > + struct rte_mempool_ops *ops; > + > + ops = rte_mempool_get_ops(mp->ops_index); > + if (ops->get_hw_cap) > + return ops->get_hw_cap(mp); > + > + return -ENOENT; > +} > + RTE_FUNC_PTR_OR_ERR_RET() can be used > /* sets mempool ops previously registered by rte_mempool_register_ops. */ > int > rte_mempool_set_ops_byname(struct rte_mempool *mp, const char *name, > diff --git a/lib/librte_mempool/rte_mempool_version.map b/lib/librte_mempool/rte_mempool_version.map > index f9c079447..d92334672 100644 > --- a/lib/librte_mempool/rte_mempool_version.map > +++ b/lib/librte_mempool/rte_mempool_version.map > @@ -41,3 +41,10 @@ DPDK_16.07 { > rte_mempool_set_ops_byname; > > } DPDK_2.0; > + > +DPDK_17.08 { > + global: > + > + rte_mempool_ops_get_hw_cap; > + > +} DPDK_17.05; /usr/bin/ld: unable to find version dependency `DPDK_17.05' This should be 16.07 here