From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7EE1FA0561; Mon, 20 Apr 2020 19:34:51 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 083201D636; Mon, 20 Apr 2020 19:34:50 +0200 (CEST) Received: from mailout2.w1.samsung.com (mailout2.w1.samsung.com [210.118.77.12]) by dpdk.org (Postfix) with ESMTP id BE05F1D58F for ; Mon, 20 Apr 2020 19:34:48 +0200 (CEST) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout2.w1.samsung.com (KnoxPortal) with ESMTP id 20200420173448euoutp02b5a3cc114e335b658a803205b3892525~Hl5uRSjXl1543015430euoutp02J; Mon, 20 Apr 2020 17:34:48 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout2.w1.samsung.com 20200420173448euoutp02b5a3cc114e335b658a803205b3892525~Hl5uRSjXl1543015430euoutp02J DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1587404088; bh=T7DeKh5I+Kpuxxz5vc6cVrvqHiDkdN8J2Mf7F1UcWA8=; h=Subject:To:Cc:From:Date:In-Reply-To:References:From; b=baCcXs91Ez1DRjv9g+9gPsqWrVS3aVuOtHI6VX+zKxps+mswVacB0MKaWNBJFriwL it9bUahvgaF34CmwPZrDImwey+f5h7Cs9wyvshUgGPyiRED2saqSm5w4gydtu2luq5 e4viOl0ZN1TrvgYuOEsvkp77mdz5MOkOQgkDM1Ls= Received: from eusmges3new.samsung.com (unknown [203.254.199.245]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20200420173447eucas1p1fe26f01146d6610aa63c051828d77db6~Hl5uJyjXq1557015570eucas1p16; Mon, 20 Apr 2020 17:34:47 +0000 (GMT) Received: from eucas1p2.samsung.com ( [182.198.249.207]) by eusmges3new.samsung.com (EUCPMTA) with SMTP id 07.7F.60698.73DDD9E5; Mon, 20 Apr 2020 18:34:47 +0100 (BST) Received: from eusmtrp2.samsung.com (unknown [182.198.249.139]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20200420173447eucas1p1dab3b8fe343e3fa146cc64539e8eaa07~Hl5t2R8hQ1573515735eucas1p1J; Mon, 20 Apr 2020 17:34:47 +0000 (GMT) Received: from eusmgms2.samsung.com (unknown [182.198.249.180]) by eusmtrp2.samsung.com (KnoxPortal) with ESMTP id 20200420173447eusmtrp2168af295104c5ec057d2e4dc390f06e1~Hl5t1Lst31352913529eusmtrp2N; Mon, 20 Apr 2020 17:34:47 +0000 (GMT) X-AuditID: cbfec7f5-a0fff7000001ed1a-9e-5e9ddd37604c Received: from eusmtip2.samsung.com ( [203.254.199.222]) by eusmgms2.samsung.com (EUCPMTA) with SMTP id FC.47.07950.73DDD9E5; Mon, 20 Apr 2020 18:34:47 +0100 (BST) Received: from [106.210.88.70] (unknown [106.210.88.70]) by eusmtip2.samsung.com (KnoxPortal) with ESMTPA id 20200420173445eusmtip2e6dce44fb498da8736b6ed4699593452~Hl5rbbpbb0447704477eusmtip2R; Mon, 20 Apr 2020 17:34:44 +0000 (GMT) To: Bruce Richardson , Thomas Monjalon Cc: "Ananyev, Konstantin" , "Dumitrescu, Cristian" , Igor Russkikh , Pavel Belous , "Lu, Wenzhuo" , Marcin Wojtas , Michal Krawczyk , Guy Tzalik , Evgeny Schemeilin , Igor Chauskin , John Daley , Hyong Youb Kim , "Zhang, Qi Z" , "Wang, Xiao W" , Ziyang Xuan , Xiaoyun Wang , Guoyang Zhou , "Wei Hu (Xavier)" , "Min Hu (Connor)" , Yisen Zhuang , "Xing, Beilei" , "Wu, Jingjing" , "Yang, Qiming" , Rasesh Mody , Shahed Shaikh , "Singh, Jasvinder" , Maxime Coquelin , "Wang, Zhihong" , "Ye, Xiaolong" , Yong Wang , "Yigit, Ferruh" , Andrew Rybchenko , Olivier Matz , "dev@dpdk.org" From: Lukasz Wojciechowski Message-ID: <57e65973-6f93-69f0-af6d-890118f0cfad@partner.samsung.com> Date: Mon, 20 Apr 2020 19:34:43 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 MIME-Version: 1.0 In-Reply-To: <20200420173015.GC1719@bricha3-MOBL.ger.corp.intel.com> Content-Transfer-Encoding: 8bit Content-Language: pl X-Brightmail-Tracker: H4sIAAAAAAAAA02Sf0zUdRjH+3x/3ZfTc19Pmk/WZF7ZrC2oSfls4Y+Wo29NN1e5MRbaGd8B iwN2XyDt11CvFMJhCB1gdASRiJmewiEJYiccJVGinpwBpdyVEJzCgQyPS7u7Lyz+e32e53k/ 7/ezfXha+we3jE/LyJaMGfp0HadmbI57vz2zZqBy27PFh17AGyUDHH42ZqLQVb8ea01TBIv/ zKPwtq+JwquXRxjsb+1Soclxk0P7+BDBAx29DB6uMbP4wJKOPvstFZZXTRBs8f/E4J1ANYOO /BYGa31uGq8MdbDYuMdK4UTh62g+WcRhSfMZgqO2aho7m75l0XcjqGr85V+C7oGjBNtuXVBh YPR5/HFfAY019V0c+r/7hsHTPcUUevPzmQ1PiD1F/USsu1TAit1HXKxYMmNlRX9VLSua2r2s WNMyTImHvu6hRcf1L1TinXNOThxuyBKPeaY58bbFy23RJKrjkqX0tFzJGLPubXVq99nTdNb0 kztPFLmpPNIfVUAieBBiYejyKRJirVBHwN+8poCogzxJoK/KxiiPCQK7r5Zyc4rd05/PKo4Q CPRFKewl8OvJTSFeIiRBmeciE+JIIRHG2vbQoUW04FdDn/MoG2pwwlpoL78bZo0QD9bKkTAz wko4axkPGvD8w8FF5mtvKiOL4edyT3hnhLARJvf6whloIQr2Nh6mFY4E16CJhLxAaIqAjus3 Z0NvBEdvHqPwEvins0Gl8GPwoNlCKQIbAefMvVl1G4HeA3WzUy/ChfszXCgRLTwFJ36IUcov gav1fDgoCIvA5V2shFgExTYzrZQ1sP9TrTIdDX8VlpI528BxD3OQ6CrmnVYx75yKeedU/O9b RZh6slTKkQ0pkrw6Q3ovWtYb5JyMlOh3Mg2nSPDXd93vvHuGnAvssBOBJ7qFmoSPK7dpWX2u vMtgJ8DTukhNclywpEnW73pfMmZuN+akS7KdPMozuqWa1dXDSVohRZ8tvStJWZJxrkvxEcvy iDWJNiVO1cdOmjYFynJV5wMTo07r1u3dF/eNmTn/qg8b4I39hWULPqrsTVv+wbXWuJUbXnaO v7U1fkdt5gLVcVNnbGRC/OM7B7/8auD3IffamIP0Cvu6Y99fWeHI5vu3jEz9XeppX55/KVUW B92vrZIeSbPGWBIMm18lpoXrXynyf/KQjpFT9c89TRtl/X9GSyLw8QMAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA02SeUxTWRSHc9/WojZeaok3aOLQzLjGYsHSg0FxEp28ZCbGJRF3rfIEtQv2 tY5bxqrNKBXDiAi0BEEQFzBRBBQFNVZFZ5AobkDCIkIiDIJQt1JxaWEm4b/vnPy+c+5NjpSW 97Gh0i1Gi2A26vRKbhRT8/V+y0xtc866WUXHw+BlejMHR/rsFDQUxUKh/SOCtFYbBb2eqxQ8 e9LNQNONGgnYq9s4cPd3Ijh6r56B7IJMFr7l6sHjfi0BZ947BFW+Bwy8HcxnoDq5ioFCTzsN TzvvsVB+oISCdylLIfNSKgfp1yoQvLmST8P9q2dY8Lz0W+UPvyBobz6P4NbrOxIYfKOB24cc NBQU1XDgu3CagdK6NAp6kpOZ+T/ydalNiD/32MHytWcbWD79cwnL+/IKWd5+t4flC6q6KP74 qTqar27MkPBvbz7n+K6yJL64w8vxvbk93GLZKlWM2WS1CD8kmkTLXOVqNUSo1NGgipgdrVJH atfOidAow+fFxAv6LTsEc/i8DarE2spSOsk7eefF1HbKhpomOVCQlODZZL/3GAqwHBci0npg rANJ/f2JpO8JHo6MI4MvHNxwpBuRG15FgMfhtSSr4x8mwAq8iuT/Wcs60Cgpje2jyUBZBxMo 5LiTIsUpLZJAisNzyV3nBzbAMvwLKcnpHmIG/0Qqc/tRYHGIf6q9btpwJJj87ewYWhCEF5D3 Bz1D76RxFDlZ2kYP8yRysDz7P1aQhld29BeSu0borhGKa4TiGqHkIaYIKQSraEgwiBEqUWcQ rcYE1SaT4TLyX9uV6oGyCuToXeZGWIqUY2Qr/shZJ2d1O8RdBjciUlqpkMXH+FuyeN2u3YLZ tN5s1QuiG2n8fztGh4ZsMvlv12hZr9aotRCt1kZqI6NAOV52GN9eI8cJOouwTRCSBPP/HiUN CrUhfEnGd10YdN5ZHftzRmczu3HCv/SnnXimLLw44cjefda4V5X1WzP1Fbh/T9SMPc7PG2IX ndjsWzjVVt8YvEQvpqZQT08G+2x4H5tSFRd8Lc1XMP6TwReysr+8wnrYEDSd/fX3sPzLkY1G 13Zdm/e3KRkDG/eHXY/btlyTffRr1qNpSkZM1Kmn02ZR9x2sDxrdgwMAAA== X-CMS-MailID: 20200420173447eucas1p1dab3b8fe343e3fa146cc64539e8eaa07 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20200420173033eucas1p263547727ddcafbf5d4120070dec1e398 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20200420173033eucas1p263547727ddcafbf5d4120070dec1e398 References: <20200417215739.23180-1-l.wojciechow@partner.samsung.com> <20200420171130.GA1719@bricha3-MOBL.ger.corp.intel.com> <7682483.vcMjziH8VY@thomas> <20200420173015.GC1719@bricha3-MOBL.ger.corp.intel.com> Subject: Re: [dpdk-dev] [PATCH v1 03/17] ethdev: replace library debug flag with global one X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" W dniu 20.04.2020 o 19:30, Bruce Richardson pisze: > On Mon, Apr 20, 2020 at 07:21:32PM +0200, Thomas Monjalon wrote: >> 20/04/2020 19:11, Bruce Richardson: >>> On Mon, Apr 20, 2020 at 04:43:21PM +0200, Lukasz Wojciechowski wrote: >>>> W dniu 20.04.2020 o 16:21, Bruce Richardson pisze: >>>>> On Mon, Apr 20, 2020 at 01:37:12PM +0000, Ananyev, Konstantin wrote: >>> >>>>>> I am agree with Cristian concern here: that patch removes ability to >>>>>> enable/disable debug on particular library/PMD. If the purpose is to >>>>>> minimize number of config compile options, I wonder can't it be done >>>>>> in a slightly different way: 1. introduce gloabal RTE_DEBUG 2. keep >>>>>> actual .[c,h] files intact 3. In actual librte_xxx/meson.build file >>>>>> check if RTE_DEBUG is enabled, If yes, then enable particular debug >>>>>> flag for these libs. Something like: If dpdk_conf.get('RTE_DEBUG') == >>>>>> true dpdk_conf.set('RTE_LIBRTE_XXX_DEBUG ', 1) >>>>>> >>>>>> defines that are used by multiple libs, probably can be set in upper >>>>>> layer meson.build. >>>>>> >>>>>> That way will have global 'debug' flag, but users will still have an >>>>>> ability to enable/disable debug flags on a per lib basis via >>>>>> CFLAGS="-D..." >>>>>> >>>>>> Konstantin >>>>>> >>>>> That seems a reasonable idea to me. >>>>> >>>>> However, in this case, we don't need the RTE_DEBUG flag at all, we can >>>>> either: >>>>> >>>>> * allow each component meson.build file define its own flags after >>>>> checking get_option('debug') * have lib/meson.build and >>>>> drivers/meson.build automatically define a specific define for each >>>>> library or driver to standardize the naming. [This would save anyone >>>>> working on it from having to lookup what the define was, since it's >>>>> always e.g. RTE_DEBUG_ + library-base-name, e.g. RTE_DEBUG_LPM, >>>>> RTE_DEBUG_SCHED etc] >>>>> >>>>> Theoretically we can also do both, have the standard ones defined and >>>>> then allow a component to provide extra flags itself if so desired. >>>>> >>>>> /Bruce >>>> OK, so let's summarize how the patches should be redo: * usage of global >>>> "debug" flag for meson build stays * we standardize names of debug flags >>>> in the components to RTE_DEBUG_ + components name * debug flag enables al >>>> the RTE_DEBUG_... flags >>>> >>>> This allow to easily use both: * the debug flag - to enable all debugs * >>>> or define manually RTE_DEBUG+component name, just for debug from a single >>>> component >>>> >>>> I like Bruce's idea of adding it to the lib/meson.build and >>>> drivers/meson.build. This way they will be added to dpdk_conf meson >>>> object and written then later to rte_build_config.h before compilation >>>> stage. All the other modules will be able to use these flags. >>>> >>> Sounds good to me (obviously!), but I'd like other feedback to ensure >>> others are ok with this before you spend too much effort implementing it. >>> >>> For the drivers, the flag probably needs to include the category as well as >>> the name, e.g. RTE_DEBUG_NET_IXGBE, RTE_DEBUG_RAW_IOAT, to avoid possible >>> name confusion. Those flags can then be checked inside individual >>> meson.build files to enable other debug flags if necessary e.g. in ixgbe, >>> you could theoretically do: >>> >>> if dpdk_conf.has('RTE_DEBUG_NET_IXGBE') >>> cflags += '-DRTE_LIBRTE_IXGBE_DEBUG_RX' >>> cflags += '-DRTE_LIBRTE_IXGBE_DEBUG_TX' >>> ... >>> endif >>> >>> to enable more fine-grained control if so desired, and to maintain >>> compatibility with existing defines, again if so desired. >> Nak the nak from Cristian. >> >> We don't need all these flags. >> If the user choose to compile DPDK for debug, every debug facilities >> should be enabled. Then at runtime it is possible to enable/disable >> the interesting logs. >> If you need to disable something which is not a log, >> you can align with the log level thanks to the function rte_log_can_log. >> >> Please let's stop complicating things for the contributors and the users. >> Note: I am strong on this position. >> > Note, this means that you need to ensure all debug printouts from libs and > drivers are using the RTE_LOG macros so can be runtime controlled. I think > that may be some distance from reality right now. > > Even if we do want all debug enabled from one flag, I'm still not 100% > convinced that the existing debug flag is the way to do, since it only adds > debug info to binary without affecting code generation. OK, I see that there are different opinions on what shape the debug flag should look like. So I think, I'll hold on work on any implementation until we all agree, what do we want. @Bruce: What code generation do you have on mind? -- Lukasz Wojciechowski Principal Software Engineer Samsung R&D Institute Poland Samsung Electronics Office +48 22 377 88 25 l.wojciechow@partner.samsung.com