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 40FD041B9B; Wed, 1 Feb 2023 09:31:39 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F1B18427F5; Wed, 1 Feb 2023 09:31:35 +0100 (CET) Received: from mail-vs1-f42.google.com (mail-vs1-f42.google.com [209.85.217.42]) by mails.dpdk.org (Postfix) with ESMTP id F3E10427F5 for ; Wed, 1 Feb 2023 09:31:34 +0100 (CET) Received: by mail-vs1-f42.google.com with SMTP id s24so11459122vsi.12 for ; Wed, 01 Feb 2023 00:31:34 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=UCwg2QFOhLELElaqefFX/xb8GxBHGddWHZBOeW50NSQ=; b=RXGm9Z/wRxRFKHhiVjcPcW4oLZ4XK2nQUoeNopJijtH40z32ImFNnnn4mMgcFR5wIs 2uw3p3RdgpVhFb2sSW144wt5RABzgubHYZh6XeugTBx9A3SMfV0fQ3zzUi6b35K+UGgJ G77XH/VR7lSAEJw9NJP7pOrWgUrQ3oMjsgBE4EcByBNIzscqdf2FZIEtOYbhUyQ2P1oH 3XD7AAw6XX2puoWsdKWKxG/v5PwpboPVHQUei/zQmw6G5y2+xB/oqR2XC2rVuM3EotuQ Rwi8jJTpzucz2lDQ9ijo3rJxvtruj1Di8cqI7XHaMN/kiRqoFe+USH/q/URZYu0W9vVq wO5A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=UCwg2QFOhLELElaqefFX/xb8GxBHGddWHZBOeW50NSQ=; b=GjrbZgSNv09Lhfz3SgiI4lgImd3UTyvY4QHlmQtgm+Q3koNuZdgjpCges82Fbz9RBy wS3vrnYgv/KwiCjvLVeK/t+ZRjI8UKDmYR7X41BJTrfB+uIfinAD7yOTTIrPytvRYINc TxEkBvV75ytoxi3TRzubxv5qds3BcBRzTdlGSIYPMDa60ABKxmoPrUd7TPkBeqTRabOh g4dy41GaIyQF3Q2AyxrUDXrSQvz4n5XD6FBigRacdu9Xmefr99mXpV/wcLPOa4UB4+TZ l8GfsAXcOUn/vekndZTHT+SpfY522UFt6w+wOkumjgLr7d2PXT0833CSCjxBD91yOMi8 R1CQ== X-Gm-Message-State: AO0yUKXJkwhaK5q2l2zNFXDiu6VKRtd0hy+ujVBzKH9e0uOARTEKb6F+ JyOkLQbg+PeE5gOKMzih6Ub1+IRZ3P4v+EPIAWE= X-Google-Smtp-Source: AK7set8emlvOS88itp69hZ02gHgyUhVZGBSOtE6Cw4/uoWopiCEyHDL7hSaMh+zOF41pgUBfBoFwyLevoBLv7zDkgI0= X-Received: by 2002:a05:6102:3ec3:b0:3f7:fd86:fabf with SMTP id n3-20020a0561023ec300b003f7fd86fabfmr359357vsv.66.1675240294303; Wed, 01 Feb 2023 00:31:34 -0800 (PST) MIME-Version: 1.0 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> <654437f5-2ee7-9d9e-adc9-20c6d4563242@amd.com> In-Reply-To: <654437f5-2ee7-9d9e-adc9-20c6d4563242@amd.com> From: Jerin Jacob Date: Wed, 1 Feb 2023 14:01:08 +0530 Message-ID: Subject: Re: [EXT] Re: [PATCH v6 2/6] ethdev: add trace points for ethdev (part one) To: Ferruh Yigit Cc: Ankur Dwivedi , "dev@dpdk.org" , David Marchand , Jerin Jacob Kollanukkaran , Andrew Rybchenko , "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" Content-Type: text/plain; charset="UTF-8" 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 Wed, Feb 1, 2023 at 3:50 AM Ferruh Yigit wrote: > > On 1/31/2023 6:46 PM, Jerin Jacob wrote: > > On Wed, Feb 1, 2023 at 12:09 AM Ferruh Yigit wrote: > >> > >> 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 > > > > rte_ethdev_trace_fp_burst.h will be installed through ninja install > > and application does not need to directly include this. So this scheme > > is OK. Right? I dont see any downside. > > > > Right. It is installed automatically with above meson change, and it is > included by 'rte_ethdev.h', so user doesn't need to include it > explicitly. Overall there is no functional problem here. > > Only this header file needs to be included (directly or indirectly) by > every application that use ethdev. I would much prefer to have an > internal header but not able to because of technical reasons (inline > functions). > After lots of effort we did to hide as much ethdev internals as we can, > now we are exposing some new trace stuff to user. > > As we can't prevent header to be public, I am just questioning below > options to reduce exposure of this header, hoping that it may lead a > better solution. Yes. All non-inline function can goto internal header file. I think, it make sense to have separated header file _fp functions using inline functions to avoid cluttering main 'rte_ethdev.h' file. > disable trace calls in inline functions on compile time, possibly > with existing 'RTE_ETHDEV_DEBUG_*' macro Disabling trace calls to inline functions, already possible with "enable_trace_fp" build option. So it will be possible to wrap around that to virtually to disable > > > > >> 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 >