[dpdk-dev] [PATCH v4 4/7] mempool: get the mempool capability

santosh santosh.shukla at caviumnetworks.com
Mon Sep 4 16:44:39 CEST 2017


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.shukla at caviumnetworks.com>
>> Signed-off-by: Jerin Jacob <jerin.jacob at 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. 



More information about the dev mailing list