From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5F9F841BAA; Thu, 2 Feb 2023 09:57:00 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id EDFCD42D33; Thu, 2 Feb 2023 09:56:59 +0100 (CET) Received: from NAM04-BN8-obe.outbound.protection.outlook.com (mail-bn8nam04on2089.outbound.protection.outlook.com [40.107.100.89]) by mails.dpdk.org (Postfix) with ESMTP id CB56D406A2 for ; Thu, 2 Feb 2023 09:56:58 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=HzAkr96NChCFPd6Tts3o8VP3QktoDJmwQj9TVVRldZbACXWothEMDQjzRThs8/L2sejBCq1EKVfmujBr3b5F7Ekgzt2AsXMy1t9mFH5AL11Jr03dtj/WRtiCDoE+hm9LBGnUrxqEi9VZLRNGJZoaH9kEZjUV3GHpmsLTD13/9a0pyZYBXCFdrBjl9jcxTZRs0127R8rVgndi47/kmuGneF+Oft4L75VQymMMV/ttBQEljoi0ZiMza/zj/8lucigFwiPSP7U2sOFENK5TAFFXhzcsZ752KN9XhwohkS/1dM24te8Pe6YP4OddGVIXTyn2+GvJXCKY8PUi5juper+T8w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=8CuCcZ/9y6ZXIFALrzeE8NAc9gBnCyU1ze7s4YxzFWI=; b=XRMQLRo49Zic6F+B7lKUvuQAx98ZJ6zI65PQnxMGoaHg0/pXkBQ5AvR9tjhGDMrR1Xn1SoIVWT3/buQekus0mhi10SkwxSqT+2dur07zW2F/tGsOm1wN+8teokvTMWuwIEEu0okoWF0r/oQMhQIIHrkAfbe3DskWSaZdU0Nk0PWyFCffysz9aEiSrzXlMK9AjXtX8F06o8/LPbk/TdqYpcPthmtp7b9FC3BbKndw3MZRGqyfYbx4N85SgRcrQDLuC5Bdsw6bcLfnuR4HS9HKx2svYkXxaUU2JqSCKlRjfZxoYi1kdOKRCQS7spDVRpCgdg/yW/1NV88H/LrMXw7ssg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=8CuCcZ/9y6ZXIFALrzeE8NAc9gBnCyU1ze7s4YxzFWI=; b=CzEJd33D4sNcuOIYIjNJEM5StrVnnjCtvDuE3C1DeotUQxxJjV6kSHt6GAKv1BNmLnPQ6pIvS1CiSxYfCZuq9TRkU70fxrnKnh26SlFTRapUPkWZVUSeu+TSAiM2XpumksGcGFW8iLstFk506hvOT0yB1gnYoeDYDja7XM231EE= Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com; Received: from CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) by SA0PR12MB4541.namprd12.prod.outlook.com (2603:10b6:806:9e::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6064.25; Thu, 2 Feb 2023 08:56:56 +0000 Received: from CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::4807:1f44:5e04:e05a]) by CH2PR12MB4294.namprd12.prod.outlook.com ([fe80::4807:1f44:5e04:e05a%8]) with mapi id 15.20.6064.024; Thu, 2 Feb 2023 08:56:56 +0000 Message-ID: Date: Thu, 2 Feb 2023 08:56:42 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.6.1 Content-Language: en-US To: Ankur Dwivedi , "dev@dpdk.org" , David Marchand , Jerin Jacob Kollanukkaran , Andrew Rybchenko Cc: "thomas@monjalon.net" , "mdr@ashroe.eu" , "orika@nvidia.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 , "shepard.siegel@atomicrules.com" , "ed.czeck@atomicrules.com" , "john.miller@atomicrules.com" , "ajit.khaparde@broadcom.com" , "somnath.kotur@broadcom.com" , "Maciej Czekaj [C]" , Shijith Thotton , Srisivasubramanian Srinivasan , Harman Kalra , "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 , Kiran Kumar Kokkilagadda , Sunil Kumar Kori , Satha Koteswara Rao Kottidi , Liron Himi , "zr@semihalf.com" , Radha Chintakuntla , Veerasenareddy Burru , Sathesh B Edara , "matan@nvidia.com" , "viacheslavo@nvidia.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 , Shahed Shaikh , Devendra Singh Rawat , "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" , "mb@smartsharesystems.com" References: <20230112112140.807233-1-adwivedi@marvell.com> <20230120084059.2926575-1-adwivedi@marvell.com> <20230120084059.2926575-3-adwivedi@marvell.com> <614e22c6-8485-0e8d-742e-b3d100f96468@amd.com> <91a635b5-3fe5-b47d-d8ba-44f9b1614bf9@amd.com> From: Ferruh Yigit Subject: Re: [EXT] Re: [PATCH v6 2/6] ethdev: add trace points for ethdev (part one) In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO4P265CA0009.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:2ad::12) To CH2PR12MB4294.namprd12.prod.outlook.com (2603:10b6:610:a9::11) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: CH2PR12MB4294:EE_|SA0PR12MB4541:EE_ X-MS-Office365-Filtering-Correlation-Id: dc72aede-2402-4d53-7d7c-08db04fb6f5a X-LD-Processed: 3dd8961f-e488-4e60-8e11-a82d994e183d,ExtAddr X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: oCJwGnQ9VAmReeRPmnXLwDjkHkwlm1BQKA6q+4erD0w6qY6w8thMVw2q7gs7wqtsDdGfgw3WKEqTGnFxqt0ND8G8+NMCQX0+npzat/NmHYycUntjALgMlvI2peNnGWws81WAePYQBIxPweTxjFlBW0dEoe/D+RwentUf1TOkqy/OFeTP+VA4EXw1pTTBiyPXh5PSbm8VbjU+IOvLRoMWGlcrt8mu7xS8K06W6p0QBJbgUuKp4VcYwAI44iJiRMiqRmTszbrOYFEHTQws36akILn0swtnR3hdmRaYCpu2QAPxag2JZtpkvCjJVpmsEJY3h5yu3Btdp8+6cNRIR4AkpMm/oB/cANkCZp4yQYrkj6ph8w8x302xxPCeMzpyAi0mbf7+/I5+kwewAp8s9yJc+j6TR8LsLo2VhouOXfYCDpocRONyWGTwAu9vxR5Ypg7CeSDbEZLEobyIc8L0bLotn/hVVbmVTwqBKtz3G4vgBGXSqJ37RbdOm/jk7HNs7RPruccpx2t5TRDN4ef6JS++rIAfTpm+G8Dy2NDMei4LhYzkj6x2xxRs6S2DtJ4wJMtz5tVxf2iTcMYU2pN01rtYLWFnbQXoGdMhcs/JlhSnKPTLMtRF3AZQPB/ZSY1pQazn8PbPwCEIEoCgJiDEfA9tomeQ5BRd7inXcs4vvV4vzZx4jNhWxszzMU61uvFvfIxBuLHGuauY3PNf2P614aNIRUt/oZOGmovIRYf9JBQCWeA= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:CH2PR12MB4294.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230025)(4636009)(366004)(376002)(136003)(39860400002)(346002)(396003)(451199018)(7336002)(7416002)(8936002)(30864003)(7406005)(7366002)(2616005)(54906003)(110136005)(5660300002)(316002)(4326008)(4001150100001)(2906002)(41300700001)(8676002)(6486002)(66476007)(6512007)(6666004)(478600001)(26005)(66946007)(6506007)(186003)(66556008)(53546011)(83380400001)(38100700002)(44832011)(45080400002)(31686004)(86362001)(31696002)(36756003)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?NGQyOFE3YmFKamw3TFZiNjBTYXNIeDV3K3M5TFpDQWx4eFhMQWc4MkZ6c09L?= =?utf-8?B?YmtGNTNmbWhYdHUrdTRnQlEzNTVSZmJFYUhxUDR2Q21oWmlUMjR4dk1jNHVi?= =?utf-8?B?ZVc2MFNyM1JJNTRrK2JuaDZMaGszc3o1RWhvenRCOUE3TE41bVFSOEVNQytE?= =?utf-8?B?WFBPNENrbCtNbzd3TEFWMzB2aENHRlp4NFdNNmJuMEY0bmk3KzlyaC9ibUV6?= =?utf-8?B?eTJsTm5EMHNIdCsyRVFsZGc3Q0RobDFIZnROQjNDQUNXVC9LU3U3aVpNUTEx?= =?utf-8?B?aUIvd05sQk9teTVWTHYwMmNuT2Jta1U3SHZ3cU9QNllValJvQzVuc2pRRy9M?= =?utf-8?B?MkRsdmZrWGp1akZVVVNkUTJFRW9CT3kyQkp4Uzk0TWZOU2hPK2pHZUt2Lzh6?= =?utf-8?B?UlhDMXRrNGlYbExENVFXV0lXcGVCazlZQ3RJV1NPYXo2cGhiOGZmK0dTS0xi?= =?utf-8?B?S2lTVzJXb08vaVkzSEpuWGliOXBNdmtkV3VIVlIwb0djSUhnbEFjbUQ4NkNY?= =?utf-8?B?NFEya0ZURDZMeFRYdFN2bWlic1NnRFEyU1A1L1M5ZDFuSEUzUE5hSlpTUG5X?= =?utf-8?B?MzZIRExvUUtoT1VWUXY0cTlSRjRyRm5laWxlMGR1cnZZck9qZkdsa2VoT2Mv?= =?utf-8?B?bXRhc3BQUDFwVnVEbGt1dEN1Z1FHMTcwOUpWMVVObVdla3VNTEIvbUV4aFdj?= =?utf-8?B?SDJDN2pCakxEMnFMNUpmd1FzVUwxUzlhT2xSaDRodkVXMnZqN1NUNVBPMGZo?= =?utf-8?B?TFFBT2c3eXhIQ3E4a056UkRweEJZZG54b3ViamUzMjBpNDA1V214UHdxOHdD?= =?utf-8?B?UzZVOVA4dHpuUzNsNnJOVXpFVTR3OWtqRFRkaHNCUnZhQi90bEdtS0hjTHpk?= =?utf-8?B?ODVTMG1pK1RsTHllWWNZWFFEV0loVm16KzVCWkdqZmthaEljQ3FnWjVoSlpX?= =?utf-8?B?WStkRGxxZ2h4N2Zxd1pHOVE0K1FyZU9xQkt5ZFptd1o1Qjd2THBYNXFhYjRC?= =?utf-8?B?TlNaWlRHaDZMUVNVaHJQWW5qdTA3NVpGRisxU1pTcjFLcS9ENjN4TGc4UXN4?= =?utf-8?B?WU5DZUJBb0lHUms3aVBkekpaY1dRdXlCeThHdjdOSnFNbmNDUk9lUkswa1VB?= =?utf-8?B?Q2YvWVZyK212S0lLejUxd25QcDZiSGZHQlpkdHlLUGRWY1l0eHlVTWhuOENS?= =?utf-8?B?clVIWkR4ekZYM29sQk5TUDJ6dXZMZlJsYitmZzJCOUh1V3NpMURMa3UxeVpm?= =?utf-8?B?dXJjUG42M0VQS0dJUVVwa2luTEMycW9tT3RJZXVJOERBU3hrL1lvNUdXQjUy?= =?utf-8?B?dDViRko0Q21wSjlLQkc3VE9Na1F4RitKb2hTMEZKd2RpZHNKN3kwN1lKVGFF?= =?utf-8?B?cDRGMThxekNMV3FYZk55VWFjODhMTHI1QjBOZ1gvVGxENmhGNkZHVFAwYnVQ?= =?utf-8?B?VVlWOU83bjRqbGtUWmF3QTZUUkN5S1JSTDhtY1lIWE1zQ2FoOWxoS0Vmbm0v?= =?utf-8?B?ekZWUy90Y0dXaVplaG1mWnBOQWlLb1MwUmQ2dCtra1Q4SVRKaWZnTW1oMmNp?= =?utf-8?B?THgvZWlJeEpFOFQ3NEZtbGlzalJUdTZGUnlHRDV2SGJTYXJvNE5ROHRqbkFL?= =?utf-8?B?UlpPekFyZWZWWVE4YlJFOE5aN0tuSTZveTRXSVNVSVR6L3pNVGtaemZpQnZC?= =?utf-8?B?MTBsekVUQU5GcEVzTDVUUGU4aUZWWGk5OW4wbWVGN29JT0I0ZHNCSlYrMDVy?= =?utf-8?B?SjBsem9wTUpwWEJxVkYwb2t5dkUvSE1Fc3cvMHpGRmh3cDBzOXVEc0labyt0?= =?utf-8?B?T3hrRDUzTlhYVXZoWDlQVDcvTEIxakFGQmFrcjNYMlY5UTdTdEhFck9HVUNu?= =?utf-8?B?ck1nelpYVEZ0Wll3U1MrcmFpVXY5d1dvOUJUWFExS3NwcVJpbDNSU3VIem5k?= =?utf-8?B?VGVpSmUwakRVSnhUbGNRRXNVN2duVjlTMjU1NGFhcnFwT1YyU3Zzei83T1Rv?= =?utf-8?B?cjJoN0FTMGVqc2kzSU1rTm5oa1FockpsQ0lHdEljRFY2RUNPQXBScW8xblJa?= =?utf-8?B?SDIzRGlQcHErSVlBRzVkemdFQndMQTAvVzR4VTRPWHBrZExleVVXVmE3NXFz?= =?utf-8?Q?8MCQmwECxyYlnPC8ro/GJoiIw?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: dc72aede-2402-4d53-7d7c-08db04fb6f5a X-MS-Exchange-CrossTenant-AuthSource: CH2PR12MB4294.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2023 08:56:55.7373 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: 9kd3ltVrJIVKGQw3OfGyfoOE20CAAH/kcTnTK5fwCfLVyHeVc0xsH4a5EMvzAnbr X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR12MB4541 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2/1/2023 3:42 PM, Ankur Dwivedi wrote: > > >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Wednesday, February 1, 2023 12:08 AM >> To: Ankur Dwivedi ; dev@dpdk.org; David >> Marchand ; Jerin Jacob Kollanukkaran >> ; Andrew Rybchenko >> >> Cc: thomas@monjalon.net; mdr@ashroe.eu; orika@nvidia.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 >> ; shepard.siegel@atomicrules.com; >> ed.czeck@atomicrules.com; john.miller@atomicrules.com; >> ajit.khaparde@broadcom.com; somnath.kotur@broadcom.com; Jerin Jacob >> Kollanukkaran ; Maciej Czekaj [C] >> ; Shijith Thotton ; >> Srisivasubramanian Srinivasan ; Harman Kalra >> ; 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 ; Kiran Kumar Kokkilagadda >> ; Sunil Kumar Kori ; Satha >> Koteswara Rao Kottidi ; Liron Himi >> ; zr@semihalf.com; Radha Chintakuntla >> ; Veerasenareddy Burru ; >> Sathesh B Edara ; matan@nvidia.com; >> viacheslavo@nvidia.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 ; Shahed Shaikh >> ; Devendra Singh Rawat >> ; andrew.rybchenko@oktetlabs.ru; >> 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; >> mb@smartsharesystems.com >> Subject: Re: [EXT] Re: [PATCH v6 2/6] ethdev: add trace points for ethdev (part >> one) >> >> On 1/30/2023 4:01 PM, Ankur Dwivedi wrote: >> >> <...> >> >>>>> diff --git a/lib/ethdev/meson.build b/lib/ethdev/meson.build index >>>>> 39250b5da1..f5c0865023 100644 >>>>> --- a/lib/ethdev/meson.build >>>>> +++ b/lib/ethdev/meson.build >>>>> @@ -24,6 +24,7 @@ headers = files( >>>>> 'rte_ethdev.h', >>>>> 'rte_ethdev_trace.h', >>>>> 'rte_ethdev_trace_fp.h', >>>>> + 'rte_ethdev_trace_fp_burst.h', >>>> >>>> Why these trace headers are public? >>>> Aren't trace points only used by the APIs, so I expect them to be >>>> internal, so applications shouldn't need them. Why they are expsed to >> user. >>> 'rte_ethdev_trace.h' can be made as internal. Not sure about >> 'rte_ethdev_trace_fp.h' and 'rte_ethdev_trace_fp_burst.h' as the tracepoints >> in fast path may be called from public inline functions. >> >> Trace calls used by inline functions needs to be public, in this case at least >> 'rte_ethdev_trace_fp_burst.h' needs to be public. >> >> Can you please at least move all trace points that are called by inline functions >> to the same file, 'rte_ethdev_trace_fp_burst.h', to reduce number of the >> header files to make public? Feel free to rename header if required. >> >> Meanwhile not sure about adding new header as dependency to end user. >> @Jerin, @Andrew, what do you think >> 1) to move these trace points to 'rte_ethdev_core.h' >> OR >> 2) disable trace calls in inline functions on compile time, possibly with existing >> 'RTE_ETHDEV_DEBUG_*' macro >> >> <...> >> >>>>> - if (port_id >= RTE_MAX_ETHPORTS) >>>>> + if (port_id >= RTE_MAX_ETHPORTS) { >>>>> + rte_eth_trace_find_next(RTE_MAX_ETHPORTS); >>>> >>>> Is there a specific reason to trace all paths, why not just keep the last one? >>> This can be kept as the function returns with RTE_MAX_ETHPORTS here. >> >> 'RTE_MAX_ETHPORTS' more like error path, that is returned when no more >> valid port left, since most of the trace doesn't record error return values, I >> thought this can be dropped as well unless there is an explicit need for it. > Ok. >> >> <...> >> >>>>> +RTE_TRACE_POINT( >>>>> + rte_eth_trace_rx_hairpin_queue_setup, >>>>> + RTE_TRACE_POINT_ARGS(uint16_t port_id, uint16_t rx_queue_id, >>>>> + uint16_t nb_rx_desc, const struct rte_eth_hairpin_conf *conf, >>>>> + int ret), >>>>> + uint32_t peer_count = conf->peer_count; >>>>> + uint32_t tx_explicit = conf->tx_explicit; >>>>> + uint32_t manual_bind = conf->manual_bind; >>>>> + uint32_t use_locked_device_memory = conf- >>>>> use_locked_device_memory; >>>>> + uint32_t use_rte_memory = conf->use_rte_memory; >>>>> + uint32_t force_memory = conf->force_memory; >>>>> + >>>>> + rte_trace_point_emit_u16(port_id); >>>>> + rte_trace_point_emit_u16(rx_queue_id); >>>>> + rte_trace_point_emit_u16(nb_rx_desc); >>>>> + rte_trace_point_emit_ptr(conf); >>>>> + rte_trace_point_emit_u32(peer_count); >>>>> + rte_trace_point_emit_u32(tx_explicit); >>>>> + rte_trace_point_emit_u32(manual_bind); >>>>> + rte_trace_point_emit_u32(use_locked_device_memory); >>>>> + rte_trace_point_emit_u32(use_rte_memory); >>>>> + rte_trace_point_emit_u32(force_memory); >>>>> + rte_trace_point_emit_int(ret); >>>> >>>> Do we need temporary variables like 'peer_count', why not directly use >> them: >>> Temporary variables are needed where the actual variable is a bitfield. >>> For example in struct rte_eth_hairpin_conf: >>> struct rte_eth_hairpin_conf { >>> uint32_t peer_count:16; >>> .... >>> uint32_t tx_explicit:1 >>> .... >>> }; >>> >>> Otherwise there is a compilation error. >>> ../lib/ethdev/rte_ethdev_trace.h:253:9: note: in expansion of macro >> ‘rte_trace_point_emit_u16’ >>> 253 | rte_trace_point_emit_u16(conf->peer_count); >>> | ^~~~~~~~~~~~~~~~~~~~~~~~ >>> ../lib/eal/include/rte_trace_point.h:369:38: error: ‘sizeof’ applied to a bit- >> field >>> 369 | mem = RTE_PTR_ADD(mem, sizeof(in)); \ >>> >> >> Got it, that is because of bitfields. But as I look the 'rte_trace_point_emit_u16' >> macro, it is like: >> >> ``` >> #define rte_trace_point_emit_u16(in) __rte_trace_point_emit(in, uint16_t) >> >> #define __rte_trace_point_emit(in, type) \ do { \ >> memcpy(mem, &(in), sizeof(in)); \ >> mem = RTE_PTR_ADD(mem, sizeof(in)); \ } while (0) ``` >> >> Here, in '__rte_trace_point_emit' macro 'type' is not used at all, if so what is >> the point of passing type? >> >> If 'sizeof(type)' used, instead of 'sizeof(in)', it would be possible to use >> bitfields directly, what do you think? > > After using 'sizeof(type)', the following compile time error is observed. > In file included from ../lib/ethdev/rte_ethdev_trace_fp_burst.h:18, > from ../lib/ethdev/rte_ethdev.h:175, > from ../lib/ethdev/rte_tm.c:8: > ../lib/ethdev/rte_ethdev_trace.h: In function ‘rte_eth_trace_rx_hairpin_queue_setup’: > ../lib/eal/include/rte_trace_point.h:378:21: error: cannot take address of bit-field ‘peer_count’ > 378 | memcpy(mem, &(in), sizeof(type)); \ > | ^ Right, you can't memcpy to bitfiled, so temporary variables are required, OK. Still, is it OK to ignore 'type' in the '__rte_trace_point_emit()'? If variable (in) is uint8_t, it won't matter if 'rte_trace_point_emit_u8', 'rte_trace_point_emit_u16' or 'rte_trace_point_emit_u32' used, they will all give same result, right? >> >> >> >> Also, as for the "struct rte_eth_hairpin_conf" sample, some of the bitfields are >> 1 bit length, does 'uint32_t' really needed for them? > > uint8_t should suffice. ack >> >> <...> >> >>>>> +RTE_TRACE_POINT_FP( >>>>> + rte_eth_trace_find_next, >>>>> + RTE_TRACE_POINT_ARGS(uint16_t port_id), >>>>> + rte_trace_point_emit_u16(port_id); >>>>> +) >>>>> + >>>> >>>> Why 'rte_eth_trace_find_next' added as fast path? >>>> Can you please add comment for all why it is added as fast path, this >>>> help to evaluate/review this later. >>> >>> There were many functions for which I was not sure about whether they >> should be slow path or fast path. I made the following assumption: >>> >>> For slow path I have chosen the function which do some setup, configure or >> write some configuration. For an example >> rte_eth_trace_tx_hairpin_queue_setup, >> rte_eth_trace_tx_buffer_set_err_callback, >> rte_eth_trace_promiscuous_enable are slow path functions. >>> >>> The functions which read data are made as fastpath functions. Also for >> functions for which I was not sure I made it as fastpath. >>> >>> For an example rte_ethdev_trace_owner_get, >> rte_eth_trace_hairpin_get_peer_port, rte_eth_trace_macaddr_get are made >> as fastpath. >>> >>> But there are few exceptions. Function like *_get_capability are made as >> slowpath. Also rte_ethdev_trace_info_get is slowpath. >>> >>> The slowpath and fastpath functions are in separate files. >> rte_ethdev_trace.h (slowpath) and rte_ethdev_trace_fp.h (fastpath). >>> >>> Please let me know if any function needs to be swapped. I will make that >> change. >>> >> >> Got it, I think most of the trace points in the 'rte_ethdev_trace_fp.h' >> are for control/helper functions like: 'rte_ethdev_trace_count_avail', >> 'rte_ethdev_trace_get_port_by_name', 'rte_eth_trace_promiscuous_get' ... >> >> I thought you did based on some analysis, that is why I asked to add that >> reasoning as code comment. >> >> >> I think we can generalize as: >> >> 1) Anything called by ethdev static inline functions are datapath, and must be >> 'RTE_TRACE_POINT_FP', like 'rte_eth_trace_call_[rt]x_callbacks', >> 'rte_ethdev_trace_[rt]x_burst', > > In this category the following functions come: > rte_eth_rx_burst > rte_eth_tx_burst > rte_eth_call_rx_callbacks (called from rte_eth_rx_burst) > rte_eth_call_tx_callbacks (called from rte_eth_tx_burst) > rte_eth_tx_buffer_count_callback (registered as error callback, called from rte_eth_tx_buffer_flush) > rte_eth_tx_buffer_drop_callback (registered as error callback) ack >> >> 2) Anything that is called in endless loop in application/sample that has >> potential impact although it may not really be datapath > > Apart from functions in category [1], I have observed the following functions in ethdev library, called in some while loop in app/examples. > rte_eth_stats_get (called in while loop in examples/qos_sched and examples/distributor) > rte_eth_macaddr_get (called in while loop in examples/bond and examples/ethtool) > rte_eth_link_get (called in for loop in examples/ip_pipeline) > rte_eth_dev_get_mtu (called in for loop in examples/ip_pipeline) > rte_eth_link_speed_to_str (called in for loop in examples/ip_pipeline) > rte_eth_dev_rx_intr_enable ( called in loop in examples/l3fwd-power) > rte_eth_dev_rx_intr_disable ( called in loop in examples/l3fwd-power) > rte_eth_timesync_read_rx_timestamp (called in loop in examples/ptpclient) > rte_eth_timesync_read_tx_timestamp (called in loop in examples/ptpclient) > rte_eth_timesync_adjust_time (called in loop in examples/ptpclient) > rte_eth_timesync_read_time (called in loop in examples/ptpclient) > rte_flow_classifier_query (called in examples/flow_classify) > rte_mtr_create (in app/test-flow-perf loop) > rte_mtr_destroy (in app/test-flow-perf loop) > rte_mtr_meter_policy_delete ((in app/test-flow-perf loop) > rte_flow_create (in app/test-flow-perf) > rte_flow_destroy (in app/test-flow-perf) > Ack, and can you please add the note within the parenthesis as a comment, whoever visits these later knows why there trace points added as fast path trace point? > Apart from the above can all other functions be moved to slowpath tracepoints? I think yes, we can start with this. At least this gives us a logic to follow. And does trace point and fast path trace points needs to be in separate header files? Can you please put first group into a header an expose it to the user, rest (as a mixture) to another header and keep it internal to ethdev? Thanks. > I think it was discussed in tech board meeting on 2022-11-30 that the functions for which the slowpath or fastpath > distinction is not clear, they should be fastpath tracepoints. > >> >> 3) Rest are regular trace points, RTE_TRACE_POINT. >> >> >> Item (2) requires some analysis and whenever you found one of them please >> comment the reasoning in the code where trace point is defined. >