[EXT] Re: [PATCH v5 0/6] add trace points in ethdev library

[Ankur Dwivedi]

Hi Ferruh,

My comments are inline.

>-----Original Message-----
>From: Ferruh Yigit <ferruh.yigit at amd.com>
>Sent: Thursday, January 12, 2023 10:33 PM
>To: Ankur Dwivedi <adwivedi at marvell.com>; dev at dpdk.org
>Cc: thomas at monjalon.net; david.marchand at redhat.com; mdr at ashroe.eu;
>orika at nvidia.com; chas3 at att.com; humin29 at huawei.com;
>linville at tuxdriver.com; ciara.loftus at intel.com; qi.z.zhang at intel.com;
>mw at semihalf.com; mk at semihalf.com; shaibran at amazon.com;
>evgenys at amazon.com; igorch at amazon.com; chandu at amd.com; Igor Russkikh
><irusskikh at marvell.com>; shepard.siegel at atomicrules.com;
>ed.czeck at atomicrules.com; john.miller at atomicrules.com;
>ajit.khaparde at broadcom.com; somnath.kotur at broadcom.com; Jerin Jacob
>Kollanukkaran <jerinj at marvell.com>; Maciej Czekaj [C]
><mczekaj at marvell.com>; Shijith Thotton <sthotton at marvell.com>;
>Srisivasubramanian Srinivasan <srinivasan at marvell.com>; Harman Kalra
><hkalra at marvell.com>; rahul.lakkireddy at chelsio.com; johndale at cisco.com;
>hyonkim at cisco.com; liudongdong3 at huawei.com; yisen.zhuang at huawei.com;
>xuanziyang2 at huawei.com; cloud.wangxiaoyun at huawei.com;
>zhouguoyang at huawei.com; simei.su at intel.com; wenjun1.wu at intel.com;
>qiming.yang at intel.com; Yuying.Zhang at intel.com; beilei.xing at intel.com;
>xiao.w.wang at intel.com; jingjing.wu at intel.com; junfeng.guo at intel.com;
>rosen.xu at intel.com; Nithin Kumar Dabilpuram <ndabilpuram at marvell.com>;
>Kiran Kumar Kokkilagadda <kirankumark at marvell.com>; Sunil Kumar Kori
><skori at marvell.com>; Satha Koteswara Rao Kottidi
><skoteshwar at marvell.com>; Liron Himi <lironh at marvell.com>;
>zr at semihalf.com; Radha Chintakuntla <radhac at marvell.com>;
>Veerasenareddy Burru <vburru at marvell.com>; Sathesh B Edara
><sedara at marvell.com>; matan at nvidia.com; viacheslavo at nvidia.com;
>longli at microsoft.com; spinler at cesnet.cz; chaoyong.he at corigine.com;
>niklas.soderlund at corigine.com; hemant.agrawal at nxp.com;
>sachin.saxena at oss.nxp.com; g.singh at nxp.com; apeksha.gupta at nxp.com;
>sachin.saxena at nxp.com; aboyer at pensando.io; Rasesh Mody
><rmody at marvell.com>; Shahed Shaikh <shshaikh at marvell.com>; Devendra
>Singh Rawat <dsinghrawat at marvell.com>; andrew.rybchenko at oktetlabs.ru;
>jiawenwu at trustnetic.com; jianwang at trustnetic.com; jbehrens at vmware.com;
>maxime.coquelin at redhat.com; chenbo.xia at intel.com;
>steven.webster at windriver.com; matt.peters at windriver.com;
>bruce.richardson at intel.com; mtetsuyah at gmail.com; grive at u256.net;
>jasvinder.singh at intel.com; cristian.dumitrescu at intel.com; jgrajcia at cisco.com;
>mb at smartsharesystems.com
>Subject: [EXT] Re: [PATCH v5 0/6] add trace points in ethdev library
>
>External Email
>
>----------------------------------------------------------------------
>On 1/12/2023 11:21 AM, Ankur Dwivedi wrote:
>> This series adds trace points for functions in the ethdev library.
>> The trace points are added in ethdev, flow, mtr and tm files.
>>
>> v5:
>>  - The rte_trace_point_emit_char_array function is renamed to
>>    rte_trace_point_emit_blob. With this function an array of
>>    any length upto 65535 bytes can be captured.
>>    For example, the following is the ctf metadata created to display
>>    a mac addr array in rte_eth_trace_macaddr_get():
>>    struct {
>>       ...
>>       uint16_t len;
>>       uint8_t mac_addr_addr_bytes[len];
>>    };
>>  - Added additional test cases for rte_eal_trace_generic_blob
>>    test case.
>>  - Capturing of return value of a function is added to tracepoint
>>    for flow, mtr and tm patches.
>>  - In ehdev patch (1/6), removed extra line. Also added rx_pkts and
>>    tx_pkts pointer in trace point.
>>
>> v4:
>>  - Adds tracepoint function to emit char array. Also adds the
>>    test case.
>>  - Resolved review comments on "ethdev: add trace point" patch.
>>    This patch is divided into 2 patches to minimize per patch
>>    size.
>>  - From the earlier version (v3), few tracepoints in ethdev,
>>    flow, mtr, tm are made as fast path tracepoints. For the
>>    tracepoint which i was unsure, i have made it as fastpath.
>>    All the fast path tracepoints can be found in
>>    rte_ethdev_trace_fp.h and rte_ethdev_trace_fp_burst.h.
>>    All the slow path tracepoints can be found in rte_ethdev_trace.h.
>>  - Capturing of return value is added to tracepoint in ethdev.
>>    For flow, mtr and tm these changes are still yet to bde done.
>>    Will do it in the next versions.
>>  - Moved the trace functions from INTERNAL to EXPERIMENTAL in
>>    version.map.
>>
>> v3:
>>  - Moved the trace functions from EXPERIMENTAL to INTERNAL in
>>    version.map.
>>  - Moved trace functions call to the end, in ethdev and flow trace.
>>  - Added code to print the input value of features in
>>    rte_eth_trace_rx_metadata_negotiate().
>>  - Added code to capture return value in flow trace.
>>
>> Ankur Dwivedi (6):
>>   eal: trace: add trace point emit for blob
>>   ethdev: add trace points for ethdev
>>   ethdev: add trace points for remaining functions
>>   ethdev: add trace points for flow
>>   ethdev: add trace points for mtr
>>   ethdev: add trace points for tm
>
>Hi Ankur,
>
>Build seems failing with set, can you please check:
>https://urldefense.proofpoint.com/v2/url?u=https-
>3A__mails.dpdk.org_archives_test-2Dreport_2023-
>2DJanuary_339696.html&d=DwICaQ&c=nKjWec2b6R0mOyPaz7xtfQ&r=ILjiNF3
>GF25y6QdHZUxMl6JrStU0MIuCtO5dMzn3Ybk&m=FhQtWKgLxBWQ7rF_fkPm7V
>K5-efg3NcVDA6cTbGsoNZmX7WvvZKas2o4xnsIxj5D&s=HDHVPZX4YsLz-
>DsRwK3mRisxOk9U8FK7fStn7gCgsXk&e=
>
>I can reproduce build failures via 32bit and debugoptimized builds.

Sure, I will check.