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 098FBA04BC for ; Thu, 17 Sep 2020 13:59:19 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id E07291D634; Thu, 17 Sep 2020 13:59:18 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id B93711D629; Thu, 17 Sep 2020 13:59:15 +0200 (CEST) IronPort-SDR: fcM0jmFol/EHAr7zO+olCBMvihTYUVo375CjQptpv/ZIUb8OpkEqP9EAdFRQT9tQbXb2smSVyw 0WtcjISzq6Tg== X-IronPort-AV: E=McAfee;i="6000,8403,9746"; a="139685315" X-IronPort-AV: E=Sophos;i="5.76,436,1592895600"; d="scan'208";a="139685315" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Sep 2020 04:59:13 -0700 IronPort-SDR: atCB8NBdHJo8kUJK/3AZBqLRwq2EBzjr25RLgAnUcEc3FU+kTcz2Nx3ceJr6RPL+VywdCDJJ7o qO662jSzAEoQ== X-IronPort-AV: E=Sophos;i="5.76,436,1592895600"; d="scan'208";a="483718048" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.251.82.126]) ([10.251.82.126]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Sep 2020 04:59:12 -0700 To: Hemant Agrawal , "Di, ChenxuX" , "hemant.agrawal@nxp.com" Cc: "sachin.saxena@nxp.com" , "stable@dpdk.org" , "dev@dpdk.org" , "Richardson, Bruce" References: <20200915024055.72103-1-chenxux.di@intel.com> <974e79a6-6a52-c84c-6ae2-cf802e551444@intel.com> <2ada3c33bf2b4dcf862e6fa4e6a97cbc@intel.com> <59ceb607-4ffc-550e-45bf-293b97248ff3@intel.com> From: Ferruh Yigit Message-ID: Date: Thu, 17 Sep 2020 12:59:08 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.2.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" On 9/17/2020 12:50 PM, Hemant Agrawal wrote: > > On 9/17/2020 5:08 PM, Ferruh Yigit wrote: >> On 9/17/2020 3:03 AM, Di, ChenxuX wrote: >>> Hi, >>> >>>> -----Original Message----- >>>> From: Ferruh Yigit >>>> Sent: Wednesday, September 16, 2020 11:29 PM >>>> To: Di, ChenxuX ; hemant.agrawal@nxp.com >>>> Cc: sachin.saxena@nxp.com; stable@dpdk.org; dev@dpdk.org; Richardson, >>>> Bruce >>>> Subject: Re: [dpdk-stable] [PATCH] net/dpaa2: fix build error about >>>> timesync >>>> functions >>>> >>>> On 9/15/2020 3:40 AM, Chenxu Di wrote: >>>>> When the build option has '-DRTE_LIBRTE_IEEE1588=1', the announce of >>>>> timesync functions will be build. >>>>> However the dpdk_conf doesn't hav RTE_LIBRTE_IEEE1588 so that the file >>>>> dpaa2_ptp.c will not be build. >>>>> It cause the build error. >>>>> This patch fixes it by adding set for dpdk_conf. >>>>> >>>>> Fixes: 184c39d16568 ("net/dpaa2: add DPRTC sub-module") >>>>> Cc: stable@dpdk.org >>>>> >>>>> Signed-off-by: Chenxu Di >>>>> --- >>>>>    drivers/net/dpaa2/meson.build | 4 ++++ >>>>>    1 file changed, 4 insertions(+) >>>>> >>>>> diff --git a/drivers/net/dpaa2/meson.build >>>>> b/drivers/net/dpaa2/meson.build index 6dd0eb274..d9aadfdae 100644 >>>>> --- a/drivers/net/dpaa2/meson.build >>>>> +++ b/drivers/net/dpaa2/meson.build >>>>> @@ -17,6 +17,10 @@ sources = files('base/dpaa2_hw_dpni.c', >>>>>            'mc/dpdmux.c', >>>>>            'mc/dpni.c') >>>>> >>>>> +if '-DRTE_LIBRTE_IEEE1588=1' in get_option('c_args') >>>> >>>> The "RTE_LIBRTE_IEEE1588=1" can fail, >>>> all places looking for "#ifdef RTE_LIBRTE_IEEE1588", so a "-Dc_args=- >>>> DRTE_LIBRTE_IEEE1588" is more likely, but why not "-Dc_args=- >>>> DRTE_LIBRTE_IEEE1588=666" >>>> >>> >>> Yes, I will change it >>> >>>>> + dpdk_conf.set('RTE_LIBRTE_IEEE1588', 1) endif >>>>> + >>>>>    if dpdk_conf.has('RTE_LIBRTE_IEEE1588') >>>>>        sources += files('mc/dprtc.c') >>>>>        sources += files('dpaa2_ptp.c') >>>>> >>>> >>>> Can't we just remove the conditional build: >>>> >>>>    -if dpdk_conf.has('RTE_LIBRTE_IEEE1588') >>>>    -       sources += files('mc/dprtc.c') >>>>    -       sources += files('dpaa2_ptp.c') >>>>    -endif >>>>    +sources += files('mc/dprtc.c') >>>>    +sources += files('dpaa2_ptp.c') >>> >>> The announce of timesync functions are in the #define >>> DRTE_LIBRTE_IEEE1588 >>> While the define of the functions are in the file 'dpaa2_ptp.c'. >>> So they should be both build or not build by whether the build option >>> -DRTE_LIBRTE_IEEE1588=1 or not. >>> So it seems not a good idea that remove the conditional. >>> >> >> timesyncs_* dev_ops functions defined but not used is not big problem, >> only can increase the library size. >> >> I believe more concern is on: >> 'RTE_PMD_REGISTER_DPAA2_OBJECT(dprtc, rte_dpaa2_dprtc_obj);' >> which looks like register function to run in constructor when >> 'dpaa2_ptp.c' is compiled, but not sure affect of it. >> It can be possible to wrap that call with 'RTE_LIBRTE_IEEE1588' ifdef, >> that should work. >> >> It is preferred to remove the compile time flag instead of finding >> ways to make it work. >> >> Let's wait for the dpaa2 maintainers' response, perhaps they can come >> with a smart way to remove the compile time flag. > > Hi Ferruh, > >    enabling  IEEE1588 causes some performance drop in the dpaa2 > performance. That is the reason, we have kept this code in compile time > flag. > > > However, we will work in future to make it run-time configurable but > that will require some code restructuring and it will be a moderate size > work. > OK, thanks Hemant. At least can it be possible to remove it from the build files, what do you think about wrapping those two files (or their relevant parts) with 'RTE_LIBRTE_IEEE1588' ifdef and remove the checks from meson file?