Re: [PATCH v1 1/2] bbdev: add new function to dump debug information

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 7/2/24 20:55, Chautru, Nicolas wrote:
> Hi Hemant,
> 
>> -----Original Message-----
>> From: Hemant Agrawal <hemant.agrawal@oss.nxp.com>
>> Sent: Tuesday, July 2, 2024 3:54 AM
>> To: Chautru, Nicolas <nicolas.chautru@intel.com>; dev@dpdk.org;
>> maxime.coquelin@redhat.com
>> Cc: hemant.agrawal@nxp.com; Marchand, David
>> <david.marchand@redhat.com>; Vargas, Hernan
>> <hernan.vargas@intel.com>
>> Subject: Re: [PATCH v1 1/2] bbdev: add new function to dump debug
>> information
>>
>> Hi Nicolas,
>>
>>       Few comments inline.
>>
>> On 02-07-2024 04:04, Nicolas Chautru wrote:
>>> This provides a new API to dump more debug information related to the
>>> status on a given bbdev queue.
>>> Some of this information is visible at bbdev level.
>>> This also provides a new option dev op, to print more information at
>>> the lower PMD level.
>>> This helps user to troubleshoot issues related to previous operations
>>> provided into a queue causing possible hard-to-debug negative
>>> scenarios.
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>> ---
>>>    lib/bbdev/rte_bbdev.c     | 214
>> ++++++++++++++++++++++++++++++++++++++
>>>    lib/bbdev/rte_bbdev.h     |  41 ++++++++
>>>    lib/bbdev/rte_bbdev_pmd.h |   9 ++
>>>    lib/bbdev/version.map     |   4 +
>>>    4 files changed, 268 insertions(+)
>>>
>>> diff --git a/lib/bbdev/rte_bbdev.c b/lib/bbdev/rte_bbdev.c index
>>> 13bde3c25b..81c031fc09 100644
>>> --- a/lib/bbdev/rte_bbdev.c
>>> +++ b/lib/bbdev/rte_bbdev.c
>>> @@ -1190,3 +1190,217 @@ rte_bbdev_enqueue_status_str(enum
>> rte_bbdev_enqueue_status status)
>>>    	rte_bbdev_log(ERR, "Invalid enqueue status");
>>>    	return NULL;
>>>    }
>>> +
>>> +
>>> +int
>>> +rte_bbdev_queue_ops_dump(uint16_t dev_id, uint16_t queue_id, FILE
>> *f)
>>> +{
>>> +	struct rte_bbdev_queue_data *q_data;
>>> +	struct rte_bbdev_stats *stats;
>>> +	uint16_t i;
>>> +	struct rte_bbdev *dev = get_dev(dev_id);
>>> +
>>> +	VALID_DEV_OR_RET_ERR(dev, dev_id);
>>> +	VALID_QUEUE_OR_RET_ERR(queue_id, dev);
>>> +	VALID_DEV_OPS_OR_RET_ERR(dev, dev_id);
>>> +	VALID_FUNC_OR_RET_ERR(dev->dev_ops->queue_ops_dump,
>> dev_id);
>>> +
>>> +	q_data = &dev->data->queues[queue_id];
>>> +
>>> +	if (f == NULL)
>>> +		return -EINVAL;
>>> +
>>> +	fprintf(f, "Dump of operations on %s queue %d\n",
>>> +			dev->data->name, queue_id);
>>> +	fprintf(f, "  Last Enqueue Status %s\n",
>>> +			rte_bbdev_enqueue_status_str(q_data-
>>> enqueue_status));
>>> +	for (i = 0; i < 4; i++)
>>
>> It shall be RTE_BBDEV_ENQ_STATUS_SIZE_MAX instead of 4 ?
> 
> Thanks, I can update this in the v2.
> 

Hi Nicolas,

could you please send a v2 addressing Hemant's comments?

Thanks,
Maxime