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 E078EA04B5 for ; Wed, 30 Sep 2020 17:09:27 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A326E1DB53; Wed, 30 Sep 2020 17:09:26 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id BD1D51D524; Wed, 30 Sep 2020 17:09:22 +0200 (CEST) IronPort-SDR: bTL2CcGF/lM4/+u9BB1yo8GgLi4Nj9s+jv/A3E8Aabjjupx+dRXwabdh0RjCezE/L1OKIwkZDC Z0jdGHRbMbLQ== X-IronPort-AV: E=McAfee;i="6000,8403,9759"; a="226613110" X-IronPort-AV: E=Sophos;i="5.77,322,1596524400"; d="scan'208";a="226613110" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Sep 2020 08:09:20 -0700 IronPort-SDR: kGehzV3Cxqtt0vZuI+x/KGSCdena4zr6aZf10YwU74vFlf8tXTZqjIdHNBjmlUqGNowjH+gd9h NcwOSkd8I7pQ== X-IronPort-AV: E=Sophos;i="5.77,322,1596524400"; d="scan'208";a="312619620" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.213.229.39]) ([10.213.229.39]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 30 Sep 2020 08:09:19 -0700 To: hemant.agrawal@nxp.com, "Di, ChenxuX" 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: <183c1c29-810f-3da1-162c-4e87eef39c55@intel.com> Date: Wed, 30 Sep 2020 16:09:15 +0100 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 4:40 PM, Hemant Agrawal wrote: > On 9/17/2020 5:29 PM, Ferruh Yigit wrote: >> 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? >> > ok. Hi Hemant, Chenxu, Can you send a new version of the patch as discussed above? Thanks, ferruh