Hi Santosh, On Wed, 21 Jun 2017 17:32:45 +0000, Santosh Shukla <santosh.shu...@caviumnetworks.com> 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 <santosh.shu...@caviumnetworks.com> > Signed-off-by: Jerin Jacob <jerin.ja...@caviumnetworks.com>
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
