Re: [PATCH v2 3/3] baseband/acc: add internal logging

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

Hi Nicolas,

On 2/28/25 6:28 PM, Chautru, Nicolas wrote:
> Hi Maxime,
> 
> I believe that for 25.03 we can just apply the first 2 commits of the serie.
> 
> For the last commit, it will take more time to try your suggestion out, we can do this in subsequent release.
> 
> Does that sound okay with you?
Yes, that sounds good to me.

Thanks,
Maxime

> Thanks
> Nic
> 
>> -----Original Message-----
>> From: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Sent: Friday, February 28, 2025 12:52 AM
>> To: Stephen Hemminger <stephen@networkplumber.org>; Chautru, Nicolas
>> <nicolas.chautru@intel.com>
>> Cc: dev@dpdk.org; hemant.agrawal@nxp.com; Vargas, Hernan
>> <hernan.vargas@intel.com>
>> Subject: Re: [PATCH v2 3/3] baseband/acc: add internal logging
>>
>> Hi Nicolas,
>>
>> On 2/7/25 10:52 AM, Maxime Coquelin wrote:
>>> Hi Nicolas,
>>>
>>> On 1/24/25 7:00 PM, Stephen Hemminger wrote:
>>>> On Fri, 24 Jan 2025 17:52:43 +0000
>>>> "Chautru, Nicolas" <nicolas.chautru@intel.com> wrote:
>>>>
>>>>> Hi Stephen,
>>>>>
>>>>> The commit message may be misleading, the logging interface doesn't
>>>>> change here. Note also that I reuse existing trace point framework
>>>>> for some of the error logging when relevant (see previous commit).
>>>>> Here the scope for that buffer is more limited, not a new logging
>>>>> method really (the commit message is misleading).
>>>>> The queue_ops_dump() already provides api to dump device specific
>>>>> information related to queue into file (logging in real time is not
>>>>> an option) based on information already in PMD memory.
>>>>> This new buffer is purely there to add storage for the string out of
>>>>> rte_bbdev_ops_param_string() for failed operation on that queue, so
>>>>> that extend that capture as this info is not stored by PMD.
>>>>> The name of the buffer could be renamed probably, or I could store
>>>>> copy of the actual operation instead of the string in case that
>>>>> makes a difference for you.
>>>>>
>>>>> I guess it would possible to move this to trace point but I thought
>>>>> it would be quite convoluted. That information would fits nicely in
>>>>> the queue dump capture, and this would require adding trace point
>>>>> for each operation type (I don't believe it can manage arbitrary
>>>>> string) and would be a bit of an unconventional use of trace point.
>>>>>
>>>>> Any thought?
>>>
>>> I think the introduction of trace points in patch 2 is a good
>>> addition, and could be extended further to not just emit a simple
>>> string but also the necessary values to enable debugging (basically
>>> what you write in the buffers).
>>>
>>> It would have the advantage of having the different traces to be
>>> synchronized (bbdev and acc ones), and also would have less
>>> performance impact.
>>
>> What do you think of this proposal?
>> how do we proceed for v25.03?
>>
>> Thanks,
>> Maxime
>>
>>>>> Thanks
>>>>> Nic
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Stephen Hemminger <stephen@networkplumber.org>
>>>>>> Sent: Thursday, January 23, 2025 3:24 PM
>>>>>> To: Chautru, Nicolas <nicolas.chautru@intel.com>
>>>>>> Cc: dev@dpdk.org; maxime.coquelin@redhat.com;
>>>>>> hemant.agrawal@nxp.com; Vargas, Hernan
>> <hernan.vargas@intel.com>
>>>>>> Subject: Re: [PATCH v2 3/3] baseband/acc: add internal logging
>>>>>>
>>>>>> On Thu, 23 Jan 2025 14:55:19 -0800
>>>>>> Nicolas Chautru <nicolas.chautru@intel.com> wrote:
>>>>>>> Adds internal buffer for more flexible logging.
>>>>>>>
>>>>>>> Signed-off-by: Nicolas Chautru <nicolas.chautru@intel.com>
>>>>>>
>>>>>> Inventing another device specific error log seems like a short
>>>>>> sighted concept.
>>>>>> Why doesn't existing DPDK logging work well enough?
>>>>>
>>>>
>>>> My feedback is that why can't you just use DEBUG logging for this.
>>>
>>> Or indeed could use the existing logging mechanism.
>>>
>>>>
>>>
>>> Thanks,
>>> Maxime
>