Hi Olivier,
On Monday 04 September 2017 08:02 PM, Olivier MATZ wrote:
> On Tue, Aug 15, 2017 at 11:37:40AM +0530, Santosh Shukla wrote:
>> Allow mempool to advertise its capability.
>> A handler been introduced called rte_mempool_ops_get_capabilities.
>> - Upon ->get_capabilities call, mempool driver will advertise
>> capability by updating to 'mp->flags'.
>>
>> Signed-off-by: Santosh Shukla <santosh.shu...@caviumnetworks.com>
>> Signed-off-by: Jerin Jacob <jerin.ja...@caviumnetworks.com>
>> ---
>> 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 f95c01c00..d518c53de 100644
>> --- a/lib/librte_mempool/rte_mempool.c
>> +++ b/lib/librte_mempool/rte_mempool.c
>> @@ -529,6 +529,11 @@ rte_mempool_populate_default(struct rte_mempool *mp)
>> if (mp->nb_mem_chunks != 0)
>> return -EEXIST;
>>
>> + /* Get mempool capability */
>> + ret = rte_mempool_ops_get_capabilities(mp);
>> + if (ret)
>> + RTE_LOG(DEBUG, MEMPOOL, "get_capability not supported for
>> %s\n", mp->name);
>> +
> there is probably a checkpatch error here (80 cols)
for debug, line over 80 char warning acceptable, right?
anyways, I will reduce verbose to less than 80 in v5.
>> +/**
>> + * @internal wrapper for mempool_ops get_capabilities callback.
>> + *
>> + * @param mp
>> + * Pointer to the memory pool.
>> + * @return
>> + * - 0: Success; Capability updated to mp->flags
>> + * - <0: Error; code of capability function.
>> + */
>> +int
>> +rte_mempool_ops_get_capabilities(struct rte_mempool *mp);
>> +
> What does "Capability updated to mp->flags" mean?
it says that external mempool driver has updated his pool capability in
mp->flags.
I'll reword in v5.
> Why not having instead:
> int rte_mempool_ops_get_capabilities(struct rte_mempool *mp,
> unsigned int *flags);
>
> ?
No strong opinion, But Since we already passing mempool as param why not update
flag info into mp->flag.
However I see your, I guess you want explicitly highlight flag as capability
update {action}
in second param, in that case how about keeping first mempool param 'const'
like below:
int rte_mempool_ops_get_capabilities(const struct rte_mempool *mp,
unsigned int *flags);
are you ok with const change in above API.
queued for v5 after you ack/nack on above const change.
Thanks.
