Re: [EXT] Re: [PATCH 1/6] ethdev: add trace points

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

@Jerin, I'd like to know what do you think about my
question/thoughts below.

On 9/13/22 10:18, Andrew Rybchenko wrote:
> On 9/13/22 09:48, Ankur Dwivedi wrote:
>> Hi Andrew,
>>
>>> -----Original Message-----
>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>>> Sent: Monday, September 12, 2022 4:30 PM
>>> To: Ankur Dwivedi <adwivedi@marvell.com>; dev@dpdk.org
>>> Cc: thomas@monjalon.net; mdr@ashroe.eu; orika@nvidia.com;
>>> ferruh.yigit@xilinx.com; chas3@att.com; humin29@huawei.com;
>>> linville@tuxdriver.com; ciara.loftus@intel.com; qi.z.zhang@intel.com;
>>> mw@semihalf.com; mk@semihalf.com; shaibran@amazon.com;
>>> evgenys@amazon.com; igorch@amazon.com; chandu@amd.com; Igor
>>> Russkikh <irusskikh@marvell.com>; shepard.siegel@atomicrules.com;
>>> ed.czeck@atomicrules.com; john.miller@atomicrules.com;
>>> ajit.khaparde@broadcom.com; somnath.kotur@broadcom.com; Jerin Jacob
>>> Kollanukkaran <jerinj@marvell.com>; Maciej Czekaj [C]
>>> <mczekaj@marvell.com>; Shijith Thotton <sthotton@marvell.com>;
>>> Srisivasubramanian Srinivasan <srinivasan@marvell.com>; Harman Kalra
>>> <hkalra@marvell.com>; rahul.lakkireddy@chelsio.com; johndale@cisco.com;
>>> hyonkim@cisco.com; liudongdong3@huawei.com;
>>> yisen.zhuang@huawei.com; xuanziyang2@huawei.com;
>>> cloud.wangxiaoyun@huawei.com; zhouguoyang@huawei.com;
>>> simei.su@intel.com; wenjun1.wu@intel.com; qiming.yang@intel.com;
>>> Yuying.Zhang@intel.com; beilei.xing@intel.com; xiao.w.wang@intel.com;
>>> jingjing.wu@intel.com; junfeng.guo@intel.com; rosen.xu@intel.com; Nithin
>>> Kumar Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar Kokkilagadda
>>> <kirankumark@marvell.com>; Sunil Kumar Kori <skori@marvell.com>; Satha
>>> Koteswara Rao Kottidi <skoteshwar@marvell.com>; Liron Himi
>>> <lironh@marvell.com>; zr@semihalf.com; Radha Chintakuntla
>>> <radhac@marvell.com>; Veerasenareddy Burru <vburru@marvell.com>;
>>> Sathesh B Edara <sedara@marvell.com>; matan@nvidia.com;
>>> viacheslavo@nvidia.com; sthemmin@microsoft.com; longli@microsoft.com;
>>> spinler@cesnet.cz; chaoyong.he@corigine.com;
>>> niklas.soderlund@corigine.com; hemant.agrawal@nxp.com;
>>> sachin.saxena@oss.nxp.com; g.singh@nxp.com; apeksha.gupta@nxp.com;
>>> sachin.saxena@nxp.com; aboyer@pensando.io; Rasesh Mody
>>> <rmody@marvell.com>; Shahed Shaikh <shshaikh@marvell.com>; Devendra
>>> Singh Rawat <dsinghrawat@marvell.com>; jiawenwu@trustnetic.com;
>>> jianwang@trustnetic.com; jbehrens@vmware.com;
>>> maxime.coquelin@redhat.com; chenbo.xia@intel.com;
>>> steven.webster@windriver.com; matt.peters@windriver.com;
>>> bruce.richardson@intel.com; mtetsuyah@gmail.com; grive@u256.net;
>>> jasvinder.singh@intel.com; cristian.dumitrescu@intel.com;
>>> jgrajcia@cisco.com
>>> Subject: [EXT] Re: [PATCH 1/6] ethdev: add trace points
>>>
>>> External Email
>>>
>>> ----------------------------------------------------------------------
>>> On 8/4/22 16:44, Ankur Dwivedi wrote:
>>>> Add trace points for ethdev functions.
>>>>
>>>> Signed-off-by: Ankur Dwivedi <adwivedi@marvell.com>
>>>> ---
>>>
>>> [snip]
>>>
>>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c index
>>>> 1979dc0850..a6fb370b22 100644
>>>> --- a/lib/ethdev/rte_ethdev.c
>>>> +++ b/lib/ethdev/rte_ethdev.c
>>>
>>> [snip]
>>>
>>>> @@ -525,6 +536,7 @@ rte_eth_dev_owner_delete(const uint64_t
>>> owner_id)
>>>>
>>>>        rte_spinlock_unlock(&eth_dev_shared_data->ownership_lock);
>>>>
>>>> +    rte_ethdev_trace_owner_delete(owner_id, ret);
>>>
>>> I'm wondering why trace is sometimes added in the middle of the 
>>> function,
>>> but in the majority of cases it is added as the first or the last 
>>> action. Is there
>>> any logical/guidelines behind it?
>> In this case for printing the return value the trace was added at the 
>> end. I can change it if not required.
>> The logic which I used was to log at least the input arguments of a 
>> function and in some cases also log important information(according to 
>> me) if possible.For example in rte_eth_tx_buffer_count_callback() I 
>> was also logging the count at the end. Similar logic in 
>> rte_eth_link_get_nowait().
>> Please let me know your views.
> 
> The answer depends on purposes of tracing. I guess that the
> main goal is to understand what the application does. So,
> tracing without logging the result does not sound really
> useful. What's the point to see that application has tried
> to enable promiscuous mode without knowing the result if
> the attempt is successful or not? If failures are critical
> for the application functionality, hopefully it will
> result in error logging which could be used together with
> tracing to understand what happens.
> 
> If so, it drives us to tracing nearby the end of the function
> when the function really has tried to do something. If there is
> no branching there we'll have some tracing of failures as well,
> but we definitely need to see the result in the trace point.
> 
> I almost have no experience with tracing, so my thoughts
> could be wrong.

Meanwhile I've updated the patch series as "Requested Changes"
since some fixes were promissed in v2.