* [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions @ 2020-09-15 2:40 Chenxu Di 2020-09-15 3:22 ` Hemant Agrawal ` (3 more replies) 0 siblings, 4 replies; 17+ messages in thread From: Chenxu Di @ 2020-09-15 2:40 UTC (permalink / raw) To: dev; +Cc: hemant.agrawal, sachin.saxena, Chenxu Di, stable 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 <chenxux.di@intel.com> --- 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') + 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') -- 2.17.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions 2020-09-15 2:40 [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions Chenxu Di @ 2020-09-15 3:22 ` Hemant Agrawal 2020-09-16 5:12 ` Zhou, JunX W ` (2 subsequent siblings) 3 siblings, 0 replies; 17+ messages in thread From: Hemant Agrawal @ 2020-09-15 3:22 UTC (permalink / raw) To: Chenxu Di, dev; +Cc: Sachin Saxena, stable Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions 2020-09-15 2:40 [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions Chenxu Di 2020-09-15 3:22 ` Hemant Agrawal @ 2020-09-16 5:12 ` Zhou, JunX W 2020-09-16 15:29 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit 2020-09-17 8:43 ` [dpdk-dev] [PATCH v2] " Chenxu Di 3 siblings, 0 replies; 17+ messages in thread From: Zhou, JunX W @ 2020-09-16 5:12 UTC (permalink / raw) To: Di, ChenxuX, dev; +Cc: hemant.agrawal, sachin.saxena, Di, ChenxuX, stable Tested-by: Zhou, Jun <junx.w.zhou@intel.com> -----Original Message----- From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Chenxu Di Sent: Tuesday, September 15, 2020 10:41 AM To: dev@dpdk.org Cc: hemant.agrawal@nxp.com; sachin.saxena@nxp.com; Di, ChenxuX <chenxux.di@intel.com>; stable@dpdk.org Subject: [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions 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 <chenxux.di@intel.com> --- 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') + 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') -- 2.17.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH] net/dpaa2: fix build error about timesync functions 2020-09-15 2:40 [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions Chenxu Di 2020-09-15 3:22 ` Hemant Agrawal 2020-09-16 5:12 ` Zhou, JunX W @ 2020-09-16 15:29 ` Ferruh Yigit 2020-09-17 2:03 ` Di, ChenxuX 2020-09-17 8:43 ` [dpdk-dev] [PATCH v2] " Chenxu Di 3 siblings, 1 reply; 17+ messages in thread From: Ferruh Yigit @ 2020-09-16 15:29 UTC (permalink / raw) To: Chenxu Di, hemant.agrawal; +Cc: sachin.saxena, stable, dev, Bruce Richardson 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 <chenxux.di@intel.com> > --- > 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" > + 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') ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH] net/dpaa2: fix build error about timesync functions 2020-09-16 15:29 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit @ 2020-09-17 2:03 ` Di, ChenxuX 2020-09-17 11:38 ` Ferruh Yigit 0 siblings, 1 reply; 17+ messages in thread From: Di, ChenxuX @ 2020-09-17 2:03 UTC (permalink / raw) To: Yigit, Ferruh, hemant.agrawal Cc: sachin.saxena, stable, dev, Richardson, Bruce Hi, > -----Original Message----- > From: Ferruh Yigit <ferruh.yigit@intel.com> > Sent: Wednesday, September 16, 2020 11:29 PM > To: Di, ChenxuX <chenxux.di@intel.com>; hemant.agrawal@nxp.com > Cc: sachin.saxena@nxp.com; stable@dpdk.org; dev@dpdk.org; Richardson, > Bruce <bruce.richardson@intel.com> > 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 <chenxux.di@intel.com> > > --- > > 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH] net/dpaa2: fix build error about timesync functions 2020-09-17 2:03 ` Di, ChenxuX @ 2020-09-17 11:38 ` Ferruh Yigit 2020-09-17 11:50 ` Hemant Agrawal 0 siblings, 1 reply; 17+ messages in thread From: Ferruh Yigit @ 2020-09-17 11:38 UTC (permalink / raw) To: Di, ChenxuX, hemant.agrawal; +Cc: sachin.saxena, stable, dev, Richardson, Bruce On 9/17/2020 3:03 AM, Di, ChenxuX wrote: > Hi, > >> -----Original Message----- >> From: Ferruh Yigit <ferruh.yigit@intel.com> >> Sent: Wednesday, September 16, 2020 11:29 PM >> To: Di, ChenxuX <chenxux.di@intel.com>; hemant.agrawal@nxp.com >> Cc: sachin.saxena@nxp.com; stable@dpdk.org; dev@dpdk.org; Richardson, >> Bruce <bruce.richardson@intel.com> >> 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 <chenxux.di@intel.com> >>> --- >>> 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH] net/dpaa2: fix build error about timesync functions 2020-09-17 11:38 ` Ferruh Yigit @ 2020-09-17 11:50 ` Hemant Agrawal 2020-09-17 11:59 ` Ferruh Yigit 0 siblings, 1 reply; 17+ messages in thread From: Hemant Agrawal @ 2020-09-17 11:50 UTC (permalink / raw) To: Ferruh Yigit, Di, ChenxuX, hemant.agrawal Cc: sachin.saxena, stable, dev, Richardson, Bruce 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 <ferruh.yigit@intel.com> >>> Sent: Wednesday, September 16, 2020 11:29 PM >>> To: Di, ChenxuX <chenxux.di@intel.com>; hemant.agrawal@nxp.com >>> Cc: sachin.saxena@nxp.com; stable@dpdk.org; dev@dpdk.org; Richardson, >>> Bruce <bruce.richardson@intel.com> >>> 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 <chenxux.di@intel.com> >>>> --- >>>> 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. Regards, Hemant ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH] net/dpaa2: fix build error about timesync functions 2020-09-17 11:50 ` Hemant Agrawal @ 2020-09-17 11:59 ` Ferruh Yigit 2020-09-17 15:40 ` Hemant Agrawal 0 siblings, 1 reply; 17+ messages in thread From: Ferruh Yigit @ 2020-09-17 11:59 UTC (permalink / raw) To: Hemant Agrawal, Di, ChenxuX, hemant.agrawal Cc: sachin.saxena, stable, dev, Richardson, Bruce 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 <ferruh.yigit@intel.com> >>>> Sent: Wednesday, September 16, 2020 11:29 PM >>>> To: Di, ChenxuX <chenxux.di@intel.com>; hemant.agrawal@nxp.com >>>> Cc: sachin.saxena@nxp.com; stable@dpdk.org; dev@dpdk.org; Richardson, >>>> Bruce <bruce.richardson@intel.com> >>>> 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 <chenxux.di@intel.com> >>>>> --- >>>>> 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? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH] net/dpaa2: fix build error about timesync functions 2020-09-17 11:59 ` Ferruh Yigit @ 2020-09-17 15:40 ` Hemant Agrawal 2020-09-30 15:09 ` Ferruh Yigit 0 siblings, 1 reply; 17+ messages in thread From: Hemant Agrawal @ 2020-09-17 15:40 UTC (permalink / raw) To: Ferruh Yigit, Di, ChenxuX, hemant.agrawal Cc: sachin.saxena, stable, dev, Richardson, Bruce 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 <ferruh.yigit@intel.com> >>>>> Sent: Wednesday, September 16, 2020 11:29 PM >>>>> To: Di, ChenxuX <chenxux.di@intel.com>; hemant.agrawal@nxp.com >>>>> Cc: sachin.saxena@nxp.com; stable@dpdk.org; dev@dpdk.org; Richardson, >>>>> Bruce <bruce.richardson@intel.com> >>>>> 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 <chenxux.di@intel.com> >>>>>> --- >>>>>> 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [dpdk-stable] [PATCH] net/dpaa2: fix build error about timesync functions 2020-09-17 15:40 ` Hemant Agrawal @ 2020-09-30 15:09 ` Ferruh Yigit 0 siblings, 0 replies; 17+ messages in thread From: Ferruh Yigit @ 2020-09-30 15:09 UTC (permalink / raw) To: hemant.agrawal, Di, ChenxuX; +Cc: sachin.saxena, stable, dev, Richardson, Bruce 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 <ferruh.yigit@intel.com> >>>>>> Sent: Wednesday, September 16, 2020 11:29 PM >>>>>> To: Di, ChenxuX <chenxux.di@intel.com>; hemant.agrawal@nxp.com >>>>>> Cc: sachin.saxena@nxp.com; stable@dpdk.org; dev@dpdk.org; Richardson, >>>>>> Bruce <bruce.richardson@intel.com> >>>>>> 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 <chenxux.di@intel.com> >>>>>>> --- >>>>>>> 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v2] net/dpaa2: fix build error about timesync functions 2020-09-15 2:40 [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions Chenxu Di ` (2 preceding siblings ...) 2020-09-16 15:29 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit @ 2020-09-17 8:43 ` Chenxu Di 2020-09-17 11:48 ` Hemant Agrawal 2020-10-06 17:16 ` [dpdk-dev] [PATCH v3] " Ferruh Yigit 3 siblings, 2 replies; 17+ messages in thread From: Chenxu Di @ 2020-09-17 8:43 UTC (permalink / raw) To: dev Cc: junx.w.zhou, hemant.agrawal, sachin.saxena, ferruh.yigit, Chenxu Di, stable 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 <chenxux.di@intel.com> --- drivers/net/dpaa2/meson.build | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/net/dpaa2/meson.build b/drivers/net/dpaa2/meson.build index 6dd0eb274..43d625dc5 100644 --- a/drivers/net/dpaa2/meson.build +++ b/drivers/net/dpaa2/meson.build @@ -17,6 +17,12 @@ sources = files('base/dpaa2_hw_dpni.c', 'mc/dpdmux.c', 'mc/dpni.c') +foreach each:get_option('c_args') + if each.contains('-DRTE_LIBRTE_IEEE1588') + dpdk_conf.set('RTE_LIBRTE_IEEE1588', 1) + endif +endforeach + if dpdk_conf.has('RTE_LIBRTE_IEEE1588') sources += files('mc/dprtc.c') sources += files('dpaa2_ptp.c') -- 2.17.1 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/dpaa2: fix build error about timesync functions 2020-09-17 8:43 ` [dpdk-dev] [PATCH v2] " Chenxu Di @ 2020-09-17 11:48 ` Hemant Agrawal 2020-10-06 17:16 ` [dpdk-dev] [PATCH v3] " Ferruh Yigit 1 sibling, 0 replies; 17+ messages in thread From: Hemant Agrawal @ 2020-09-17 11:48 UTC (permalink / raw) To: Chenxu Di, dev Cc: junx.w.zhou, hemant.agrawal, sachin.saxena, ferruh.yigit, stable Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com> On 9/17/2020 2:13 PM, 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 <chenxux.di@intel.com> > --- > drivers/net/dpaa2/meson.build | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/net/dpaa2/meson.build b/drivers/net/dpaa2/meson.build > index 6dd0eb274..43d625dc5 100644 > --- a/drivers/net/dpaa2/meson.build > +++ b/drivers/net/dpaa2/meson.build > @@ -17,6 +17,12 @@ sources = files('base/dpaa2_hw_dpni.c', > 'mc/dpdmux.c', > 'mc/dpni.c') > > +foreach each:get_option('c_args') > + if each.contains('-DRTE_LIBRTE_IEEE1588') > + dpdk_conf.set('RTE_LIBRTE_IEEE1588', 1) > + endif > +endforeach > + > if dpdk_conf.has('RTE_LIBRTE_IEEE1588') > sources += files('mc/dprtc.c') > sources += files('dpaa2_ptp.c') ^ permalink raw reply [flat|nested] 17+ messages in thread
* [dpdk-dev] [PATCH v3] net/dpaa2: fix build error about timesync functions 2020-09-17 8:43 ` [dpdk-dev] [PATCH v2] " Chenxu Di 2020-09-17 11:48 ` Hemant Agrawal @ 2020-10-06 17:16 ` Ferruh Yigit 2020-10-08 2:24 ` Sachin Saxena (OSS) 1 sibling, 1 reply; 17+ messages in thread From: Ferruh Yigit @ 2020-10-06 17:16 UTC (permalink / raw) To: Hemant Agrawal, Sachin Saxena, Priyanka Jain; +Cc: dev, Chenxu Di, stable From: Chenxu Di <chenxux.di@intel.com> Some timesync related source files are built only when 'RTE_LIBRTE_IEEE1588' config is set, which is missing in meson. Building with '-DRTE_LIBRTE_IEEE1588' cflag will enable timesync functionality in the files that are build, but it won't build files that require 'RTE_LIBRTE_IEEE1588' config. This causes the build error. Fixing by removing config check in meson file and build all .c files by default, but wrap relevant code part with 'RTE_LIBRTE_IEEE1588' macro. Also removing 'RTE_LIBRTE_IEEE1588' ifdef around some fields of the data structures, to not require finer grained macro wraps on the functions using them. Since the registration of the function disabled with macro check, having functions compiled shouldn't affect the functionality. Fixes: 184c39d16568 ("net/dpaa2: add DPRTC sub-module") Cc: stable@dpdk.org Signed-off-by: Chenxu Di <chenxux.di@intel.com> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- drivers/net/dpaa2/dpaa2_ethdev.h | 4 ---- drivers/net/dpaa2/dpaa2_ptp.c | 2 ++ drivers/net/dpaa2/meson.build | 7 ++----- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/net/dpaa2/dpaa2_ethdev.h b/drivers/net/dpaa2/dpaa2_ethdev.h index e577355d57..94cf253827 100644 --- a/drivers/net/dpaa2/dpaa2_ethdev.h +++ b/drivers/net/dpaa2/dpaa2_ethdev.h @@ -160,7 +160,6 @@ struct dpaa2_dev_priv { uint16_t ss_offset; uint64_t ss_iova; uint64_t ss_param_iova; -#if defined(RTE_LIBRTE_IEEE1588) /*stores timestamp of last received packet on dev*/ uint64_t rx_timestamp; /*stores timestamp of last received tx confirmation packet on dev*/ @@ -169,7 +168,6 @@ struct dpaa2_dev_priv { * it corresponds to last packet transmitted */ struct dpaa2_queue *next_tx_conf_queue; -#endif struct rte_eth_dev *eth_dev; /**< Pointer back to holding ethdev */ @@ -227,7 +225,6 @@ void dpaa2_dev_free_eqresp_buf(uint16_t eqresp_ci); void dpaa2_flow_clean(struct rte_eth_dev *dev); uint16_t dpaa2_dev_tx_conf(void *queue) __rte_unused; -#if defined(RTE_LIBRTE_IEEE1588) int dpaa2_timesync_enable(struct rte_eth_dev *dev); int dpaa2_timesync_disable(struct rte_eth_dev *dev); int dpaa2_timesync_read_time(struct rte_eth_dev *dev, @@ -240,5 +237,4 @@ int dpaa2_timesync_read_rx_timestamp(struct rte_eth_dev *dev, uint32_t flags __rte_unused); int dpaa2_timesync_read_tx_timestamp(struct rte_eth_dev *dev, struct timespec *timestamp); -#endif #endif /* _DPAA2_ETHDEV_H */ diff --git a/drivers/net/dpaa2/dpaa2_ptp.c b/drivers/net/dpaa2/dpaa2_ptp.c index f58eedb314..899dd5d442 100644 --- a/drivers/net/dpaa2/dpaa2_ptp.c +++ b/drivers/net/dpaa2/dpaa2_ptp.c @@ -129,6 +129,7 @@ int dpaa2_timesync_read_rx_timestamp(struct rte_eth_dev *dev, return 0; } +#if defined(RTE_LIBRTE_IEEE1588) static int dpaa2_create_dprtc_device(int vdev_fd __rte_unused, struct vfio_device_info *obj_info __rte_unused, @@ -179,3 +180,4 @@ static struct rte_dpaa2_object rte_dpaa2_dprtc_obj = { }; RTE_PMD_REGISTER_DPAA2_OBJECT(dprtc, rte_dpaa2_dprtc_obj); +#endif diff --git a/drivers/net/dpaa2/meson.build b/drivers/net/dpaa2/meson.build index 6dd0eb274c..4312aa73f7 100644 --- a/drivers/net/dpaa2/meson.build +++ b/drivers/net/dpaa2/meson.build @@ -13,15 +13,12 @@ sources = files('base/dpaa2_hw_dpni.c', 'dpaa2_flow.c', 'dpaa2_rxtx.c', 'dpaa2_sparser.c', + 'dpaa2_ptp.c', + 'mc/dprtc.c', 'mc/dpkg.c', 'mc/dpdmux.c', 'mc/dpni.c') -if dpdk_conf.has('RTE_LIBRTE_IEEE1588') - sources += files('mc/dprtc.c') - sources += files('dpaa2_ptp.c') -endif - includes += include_directories('base', 'mc') install_headers('rte_pmd_dpaa2.h') -- 2.26.2 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/dpaa2: fix build error about timesync functions 2020-10-06 17:16 ` [dpdk-dev] [PATCH v3] " Ferruh Yigit @ 2020-10-08 2:24 ` Sachin Saxena (OSS) 2020-10-08 13:12 ` Ferruh Yigit 0 siblings, 1 reply; 17+ messages in thread From: Sachin Saxena (OSS) @ 2020-10-08 2:24 UTC (permalink / raw) To: Ferruh Yigit, Hemant Agrawal, Chenxu Di; +Cc: Priyanka Jain, dev, stable Following change should also be handled as suggested by Ferruh in earlier. diff --git a/drivers/net/dpaa2/dpaa2_ptp.c b/drivers/net/dpaa2/dpaa2_ptp.c index 9f755e84bf..0e44d4c6bf 100644 --- a/drivers/net/dpaa2/dpaa2_ptp.c +++ b/drivers/net/dpaa2/dpaa2_ptp.c @@ -178,4 +178,6 @@ static struct rte_dpaa2_object rte_dpaa2_dprtc_obj = { .create = dpaa2_create_dprtc_device, }; +#if defined(RTE_LIBRTE_IEEE1588) RTE_PMD_REGISTER_DPAA2_OBJECT(dprtc, rte_dpaa2_dprtc_obj); +#endif regards, Sachin On 06-Oct-20 10:46 PM, Ferruh Yigit wrote: > From: Chenxu Di <chenxux.di@intel.com> > > Some timesync related source files are built only when > 'RTE_LIBRTE_IEEE1588' config is set, which is missing in meson. > > Building with '-DRTE_LIBRTE_IEEE1588' cflag will enable timesync > functionality in the files that are build, but it won't build files > that require 'RTE_LIBRTE_IEEE1588' config. > This causes the build error. > > Fixing by removing config check in meson file and build all .c files by > default, but wrap relevant code part with 'RTE_LIBRTE_IEEE1588' macro. > > Also removing 'RTE_LIBRTE_IEEE1588' ifdef around some fields of the data > structures, to not require finer grained macro wraps on the functions > using them. > Since the registration of the function disabled with macro check, having > functions compiled shouldn't affect the functionality. > > Fixes: 184c39d16568 ("net/dpaa2: add DPRTC sub-module") > Cc: stable@dpdk.org > > Signed-off-by: Chenxu Di <chenxux.di@intel.com> > Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> > --- > drivers/net/dpaa2/dpaa2_ethdev.h | 4 ---- > drivers/net/dpaa2/dpaa2_ptp.c | 2 ++ > drivers/net/dpaa2/meson.build | 7 ++----- > 3 files changed, 4 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/dpaa2/dpaa2_ethdev.h b/drivers/net/dpaa2/dpaa2_ethdev.h > index e577355d57..94cf253827 100644 > --- a/drivers/net/dpaa2/dpaa2_ethdev.h > +++ b/drivers/net/dpaa2/dpaa2_ethdev.h > @@ -160,7 +160,6 @@ struct dpaa2_dev_priv { > uint16_t ss_offset; > uint64_t ss_iova; > uint64_t ss_param_iova; > -#if defined(RTE_LIBRTE_IEEE1588) > /*stores timestamp of last received packet on dev*/ > uint64_t rx_timestamp; > /*stores timestamp of last received tx confirmation packet on dev*/ > @@ -169,7 +168,6 @@ struct dpaa2_dev_priv { > * it corresponds to last packet transmitted > */ > struct dpaa2_queue *next_tx_conf_queue; > -#endif > > struct rte_eth_dev *eth_dev; /**< Pointer back to holding ethdev */ > > @@ -227,7 +225,6 @@ void dpaa2_dev_free_eqresp_buf(uint16_t eqresp_ci); > void dpaa2_flow_clean(struct rte_eth_dev *dev); > uint16_t dpaa2_dev_tx_conf(void *queue) __rte_unused; > > -#if defined(RTE_LIBRTE_IEEE1588) > int dpaa2_timesync_enable(struct rte_eth_dev *dev); > int dpaa2_timesync_disable(struct rte_eth_dev *dev); > int dpaa2_timesync_read_time(struct rte_eth_dev *dev, > @@ -240,5 +237,4 @@ int dpaa2_timesync_read_rx_timestamp(struct rte_eth_dev *dev, > uint32_t flags __rte_unused); > int dpaa2_timesync_read_tx_timestamp(struct rte_eth_dev *dev, > struct timespec *timestamp); > -#endif > #endif /* _DPAA2_ETHDEV_H */ > diff --git a/drivers/net/dpaa2/dpaa2_ptp.c b/drivers/net/dpaa2/dpaa2_ptp.c > index f58eedb314..899dd5d442 100644 > --- a/drivers/net/dpaa2/dpaa2_ptp.c > +++ b/drivers/net/dpaa2/dpaa2_ptp.c > @@ -129,6 +129,7 @@ int dpaa2_timesync_read_rx_timestamp(struct rte_eth_dev *dev, > return 0; > } > > +#if defined(RTE_LIBRTE_IEEE1588) > static int > dpaa2_create_dprtc_device(int vdev_fd __rte_unused, > struct vfio_device_info *obj_info __rte_unused, > @@ -179,3 +180,4 @@ static struct rte_dpaa2_object rte_dpaa2_dprtc_obj = { > }; > > RTE_PMD_REGISTER_DPAA2_OBJECT(dprtc, rte_dpaa2_dprtc_obj); > +#endif > diff --git a/drivers/net/dpaa2/meson.build b/drivers/net/dpaa2/meson.build > index 6dd0eb274c..4312aa73f7 100644 > --- a/drivers/net/dpaa2/meson.build > +++ b/drivers/net/dpaa2/meson.build > @@ -13,15 +13,12 @@ sources = files('base/dpaa2_hw_dpni.c', > 'dpaa2_flow.c', > 'dpaa2_rxtx.c', > 'dpaa2_sparser.c', > + 'dpaa2_ptp.c', > + 'mc/dprtc.c', > 'mc/dpkg.c', > 'mc/dpdmux.c', > 'mc/dpni.c') > > -if dpdk_conf.has('RTE_LIBRTE_IEEE1588') > - sources += files('mc/dprtc.c') > - sources += files('dpaa2_ptp.c') > -endif > - > includes += include_directories('base', 'mc') > > install_headers('rte_pmd_dpaa2.h') ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/dpaa2: fix build error about timesync functions 2020-10-08 2:24 ` Sachin Saxena (OSS) @ 2020-10-08 13:12 ` Ferruh Yigit 2020-10-09 7:48 ` Sachin Saxena (OSS) 0 siblings, 1 reply; 17+ messages in thread From: Ferruh Yigit @ 2020-10-08 13:12 UTC (permalink / raw) To: Sachin Saxena (OSS), Hemant Agrawal, Chenxu Di; +Cc: Priyanka Jain, dev, stable On 10/8/2020 3:24 AM, Sachin Saxena (OSS) wrote: [please don't top post, I moved reply to down] > > On 06-Oct-20 10:46 PM, Ferruh Yigit wrote: >> From: Chenxu Di<chenxux.di@intel.com> >> >> Some timesync related source files are built only when >> 'RTE_LIBRTE_IEEE1588' config is set, which is missing in meson. >> >> Building with '-DRTE_LIBRTE_IEEE1588' cflag will enable timesync >> functionality in the files that are build, but it won't build files >> that require 'RTE_LIBRTE_IEEE1588' config. >> This causes the build error. >> >> Fixing by removing config check in meson file and build all .c files by >> default, but wrap relevant code part with 'RTE_LIBRTE_IEEE1588' macro. >> >> Also removing 'RTE_LIBRTE_IEEE1588' ifdef around some fields of the data >> structures, to not require finer grained macro wraps on the functions >> using them. >> Since the registration of the function disabled with macro check, having >> functions compiled shouldn't affect the functionality. >> >> Fixes: 184c39d16568 ("net/dpaa2: add DPRTC sub-module") >> Cc:stable@dpdk.org >> >> Signed-off-by: Chenxu Di<chenxux.di@intel.com> >> Signed-off-by: Ferruh Yigit<ferruh.yigit@intel.com> >> --- >> drivers/net/dpaa2/dpaa2_ethdev.h | 4 ---- >> drivers/net/dpaa2/dpaa2_ptp.c | 2 ++ >> drivers/net/dpaa2/meson.build | 7 ++----- >> 3 files changed, 4 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.h b/drivers/net/dpaa2/dpaa2_ethdev.h >> index e577355d57..94cf253827 100644 >> --- a/drivers/net/dpaa2/dpaa2_ethdev.h >> +++ b/drivers/net/dpaa2/dpaa2_ethdev.h >> @@ -160,7 +160,6 @@ struct dpaa2_dev_priv { >> uint16_t ss_offset; >> uint64_t ss_iova; >> uint64_t ss_param_iova; >> -#if defined(RTE_LIBRTE_IEEE1588) >> /*stores timestamp of last received packet on dev*/ >> uint64_t rx_timestamp; >> /*stores timestamp of last received tx confirmation packet on dev*/ >> @@ -169,7 +168,6 @@ struct dpaa2_dev_priv { >> * it corresponds to last packet transmitted >> */ >> struct dpaa2_queue *next_tx_conf_queue; >> -#endif >> >> struct rte_eth_dev *eth_dev; /**< Pointer back to holding ethdev */ >> >> @@ -227,7 +225,6 @@ void dpaa2_dev_free_eqresp_buf(uint16_t eqresp_ci); >> void dpaa2_flow_clean(struct rte_eth_dev *dev); >> uint16_t dpaa2_dev_tx_conf(void *queue) __rte_unused; >> >> -#if defined(RTE_LIBRTE_IEEE1588) >> int dpaa2_timesync_enable(struct rte_eth_dev *dev); >> int dpaa2_timesync_disable(struct rte_eth_dev *dev); >> int dpaa2_timesync_read_time(struct rte_eth_dev *dev, >> @@ -240,5 +237,4 @@ int dpaa2_timesync_read_rx_timestamp(struct rte_eth_dev *dev, >> uint32_t flags __rte_unused); >> int dpaa2_timesync_read_tx_timestamp(struct rte_eth_dev *dev, >> struct timespec *timestamp); >> -#endif >> #endif /* _DPAA2_ETHDEV_H */ >> diff --git a/drivers/net/dpaa2/dpaa2_ptp.c b/drivers/net/dpaa2/dpaa2_ptp.c >> index f58eedb314..899dd5d442 100644 >> --- a/drivers/net/dpaa2/dpaa2_ptp.c >> +++ b/drivers/net/dpaa2/dpaa2_ptp.c >> @@ -129,6 +129,7 @@ int dpaa2_timesync_read_rx_timestamp(struct rte_eth_dev *dev, >> return 0; >> } >> >> +#if defined(RTE_LIBRTE_IEEE1588) >> static int >> dpaa2_create_dprtc_device(int vdev_fd __rte_unused, >> struct vfio_device_info *obj_info __rte_unused, >> @@ -179,3 +180,4 @@ static struct rte_dpaa2_object rte_dpaa2_dprtc_obj = { >> }; >> >> RTE_PMD_REGISTER_DPAA2_OBJECT(dprtc, rte_dpaa2_dprtc_obj); >> +#endif >> diff --git a/drivers/net/dpaa2/meson.build b/drivers/net/dpaa2/meson.build >> index 6dd0eb274c..4312aa73f7 100644 >> --- a/drivers/net/dpaa2/meson.build >> +++ b/drivers/net/dpaa2/meson.build >> @@ -13,15 +13,12 @@ sources = files('base/dpaa2_hw_dpni.c', >> 'dpaa2_flow.c', >> 'dpaa2_rxtx.c', >> 'dpaa2_sparser.c', >> + 'dpaa2_ptp.c', >> + 'mc/dprtc.c', >> 'mc/dpkg.c', >> 'mc/dpdmux.c', >> 'mc/dpni.c') >> >> -if dpdk_conf.has('RTE_LIBRTE_IEEE1588') >> - sources += files('mc/dprtc.c') >> - sources += files('dpaa2_ptp.c') >> -endif >> - >> includes += include_directories('base', 'mc') >> >> install_headers('rte_pmd_dpaa2.h') > > Following change should also be handled as suggested by Ferruh in earlier. > > diff --git a/drivers/net/dpaa2/dpaa2_ptp.c b/drivers/net/dpaa2/dpaa2_ptp.c > index 9f755e84bf..0e44d4c6bf 100644 > --- a/drivers/net/dpaa2/dpaa2_ptp.c > +++ b/drivers/net/dpaa2/dpaa2_ptp.c > @@ -178,4 +178,6 @@ static struct rte_dpaa2_object rte_dpaa2_dprtc_obj = { > .create = dpaa2_create_dprtc_device, > }; > > +#if defined(RTE_LIBRTE_IEEE1588) > RTE_PMD_REGISTER_DPAA2_OBJECT(dprtc, rte_dpaa2_dprtc_obj); > +#endif > I think it is already covered in the patch. Since just wrapping the macro cause a warning on unused static variable/function, I need to wrap 'rte_dpaa2_dprtc_obj' & 'dpaa2_create_dprtc_device()' too along with above macro you mentioned. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/dpaa2: fix build error about timesync functions 2020-10-08 13:12 ` Ferruh Yigit @ 2020-10-09 7:48 ` Sachin Saxena (OSS) 2020-10-09 11:22 ` Ferruh Yigit 0 siblings, 1 reply; 17+ messages in thread From: Sachin Saxena (OSS) @ 2020-10-09 7:48 UTC (permalink / raw) To: Ferruh Yigit, Hemant Agrawal, Chenxu Di; +Cc: Priyanka Jain, dev, stable Acked-by: Sachin Saxena<sachin.saxena@oss.nxp.com> On 08-Oct-20 6:42 PM, Ferruh Yigit wrote: > On 10/8/2020 3:24 AM, Sachin Saxena (OSS) wrote: > > [please don't top post, I moved reply to down] > >> >> On 06-Oct-20 10:46 PM, Ferruh Yigit wrote: >>> From: Chenxu Di<chenxux.di@intel.com> >>> >>> Some timesync related source files are built only when >>> 'RTE_LIBRTE_IEEE1588' config is set, which is missing in meson. >>> >>> Building with '-DRTE_LIBRTE_IEEE1588' cflag will enable timesync >>> functionality in the files that are build, but it won't build files >>> that require 'RTE_LIBRTE_IEEE1588' config. >>> This causes the build error. >>> >>> Fixing by removing config check in meson file and build all .c files by >>> default, but wrap relevant code part with 'RTE_LIBRTE_IEEE1588' macro. >>> >>> Also removing 'RTE_LIBRTE_IEEE1588' ifdef around some fields of the >>> data >>> structures, to not require finer grained macro wraps on the functions >>> using them. >>> Since the registration of the function disabled with macro check, >>> having >>> functions compiled shouldn't affect the functionality. >>> >>> Fixes: 184c39d16568 ("net/dpaa2: add DPRTC sub-module") >>> Cc:stable@dpdk.org >>> >>> Signed-off-by: Chenxu Di<chenxux.di@intel.com> >>> Signed-off-by: Ferruh Yigit<ferruh.yigit@intel.com> >>> --- >>> drivers/net/dpaa2/dpaa2_ethdev.h | 4 ---- >>> drivers/net/dpaa2/dpaa2_ptp.c | 2 ++ >>> drivers/net/dpaa2/meson.build | 7 ++----- >>> 3 files changed, 4 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.h >>> b/drivers/net/dpaa2/dpaa2_ethdev.h >>> index e577355d57..94cf253827 100644 >>> --- a/drivers/net/dpaa2/dpaa2_ethdev.h >>> +++ b/drivers/net/dpaa2/dpaa2_ethdev.h >>> @@ -160,7 +160,6 @@ struct dpaa2_dev_priv { >>> uint16_t ss_offset; >>> uint64_t ss_iova; >>> uint64_t ss_param_iova; >>> -#if defined(RTE_LIBRTE_IEEE1588) >>> /*stores timestamp of last received packet on dev*/ >>> uint64_t rx_timestamp; >>> /*stores timestamp of last received tx confirmation packet on >>> dev*/ >>> @@ -169,7 +168,6 @@ struct dpaa2_dev_priv { >>> * it corresponds to last packet transmitted >>> */ >>> struct dpaa2_queue *next_tx_conf_queue; >>> -#endif >>> struct rte_eth_dev *eth_dev; /**< Pointer back to holding >>> ethdev */ >>> @@ -227,7 +225,6 @@ void dpaa2_dev_free_eqresp_buf(uint16_t >>> eqresp_ci); >>> void dpaa2_flow_clean(struct rte_eth_dev *dev); >>> uint16_t dpaa2_dev_tx_conf(void *queue) __rte_unused; >>> -#if defined(RTE_LIBRTE_IEEE1588) >>> int dpaa2_timesync_enable(struct rte_eth_dev *dev); >>> int dpaa2_timesync_disable(struct rte_eth_dev *dev); >>> int dpaa2_timesync_read_time(struct rte_eth_dev *dev, >>> @@ -240,5 +237,4 @@ int dpaa2_timesync_read_rx_timestamp(struct >>> rte_eth_dev *dev, >>> uint32_t flags __rte_unused); >>> int dpaa2_timesync_read_tx_timestamp(struct rte_eth_dev *dev, >>> struct timespec *timestamp); >>> -#endif >>> #endif /* _DPAA2_ETHDEV_H */ >>> diff --git a/drivers/net/dpaa2/dpaa2_ptp.c >>> b/drivers/net/dpaa2/dpaa2_ptp.c >>> index f58eedb314..899dd5d442 100644 >>> --- a/drivers/net/dpaa2/dpaa2_ptp.c >>> +++ b/drivers/net/dpaa2/dpaa2_ptp.c >>> @@ -129,6 +129,7 @@ int dpaa2_timesync_read_rx_timestamp(struct >>> rte_eth_dev *dev, >>> return 0; >>> } >>> +#if defined(RTE_LIBRTE_IEEE1588) >>> static int >>> dpaa2_create_dprtc_device(int vdev_fd __rte_unused, >>> struct vfio_device_info *obj_info __rte_unused, >>> @@ -179,3 +180,4 @@ static struct rte_dpaa2_object >>> rte_dpaa2_dprtc_obj = { >>> }; >>> RTE_PMD_REGISTER_DPAA2_OBJECT(dprtc, rte_dpaa2_dprtc_obj); >>> +#endif >>> diff --git a/drivers/net/dpaa2/meson.build >>> b/drivers/net/dpaa2/meson.build >>> index 6dd0eb274c..4312aa73f7 100644 >>> --- a/drivers/net/dpaa2/meson.build >>> +++ b/drivers/net/dpaa2/meson.build >>> @@ -13,15 +13,12 @@ sources = files('base/dpaa2_hw_dpni.c', >>> 'dpaa2_flow.c', >>> 'dpaa2_rxtx.c', >>> 'dpaa2_sparser.c', >>> + 'dpaa2_ptp.c', >>> + 'mc/dprtc.c', >>> 'mc/dpkg.c', >>> 'mc/dpdmux.c', >>> 'mc/dpni.c') >>> -if dpdk_conf.has('RTE_LIBRTE_IEEE1588') >>> - sources += files('mc/dprtc.c') >>> - sources += files('dpaa2_ptp.c') >>> -endif >>> - >>> includes += include_directories('base', 'mc') >>> install_headers('rte_pmd_dpaa2.h') >> > > Following change should also be handled as suggested by Ferruh in > earlier. > > > > diff --git a/drivers/net/dpaa2/dpaa2_ptp.c > b/drivers/net/dpaa2/dpaa2_ptp.c > > index 9f755e84bf..0e44d4c6bf 100644 > > --- a/drivers/net/dpaa2/dpaa2_ptp.c > > +++ b/drivers/net/dpaa2/dpaa2_ptp.c > > @@ -178,4 +178,6 @@ static struct rte_dpaa2_object > rte_dpaa2_dprtc_obj = { > > .create = dpaa2_create_dprtc_device, > > }; > > > > +#if defined(RTE_LIBRTE_IEEE1588) > > RTE_PMD_REGISTER_DPAA2_OBJECT(dprtc, rte_dpaa2_dprtc_obj); > > +#endif > > > > I think it is already covered in the patch. > > Since just wrapping the macro cause a warning on unused static > variable/function, I need to wrap 'rte_dpaa2_dprtc_obj' & > 'dpaa2_create_dprtc_device()' too along with above macro you mentioned. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/dpaa2: fix build error about timesync functions 2020-10-09 7:48 ` Sachin Saxena (OSS) @ 2020-10-09 11:22 ` Ferruh Yigit 0 siblings, 0 replies; 17+ messages in thread From: Ferruh Yigit @ 2020-10-09 11:22 UTC (permalink / raw) To: Sachin Saxena (OSS), Hemant Agrawal, Chenxu Di; +Cc: Priyanka Jain, dev, stable On 10/9/2020 8:48 AM, Sachin Saxena (OSS) wrote: > On 08-Oct-20 6:42 PM, Ferruh Yigit wrote: >> On 10/8/2020 3:24 AM, Sachin Saxena (OSS) wrote: >> >> [please don't top post, I moved reply to down] >> >>> >>> On 06-Oct-20 10:46 PM, Ferruh Yigit wrote: >>>> From: Chenxu Di<chenxux.di@intel.com> >>>> >>>> Some timesync related source files are built only when >>>> 'RTE_LIBRTE_IEEE1588' config is set, which is missing in meson. >>>> >>>> Building with '-DRTE_LIBRTE_IEEE1588' cflag will enable timesync >>>> functionality in the files that are build, but it won't build files >>>> that require 'RTE_LIBRTE_IEEE1588' config. >>>> This causes the build error. >>>> >>>> Fixing by removing config check in meson file and build all .c files by >>>> default, but wrap relevant code part with 'RTE_LIBRTE_IEEE1588' macro. >>>> >>>> Also removing 'RTE_LIBRTE_IEEE1588' ifdef around some fields of the data >>>> structures, to not require finer grained macro wraps on the functions >>>> using them. >>>> Since the registration of the function disabled with macro check, having >>>> functions compiled shouldn't affect the functionality. >>>> >>>> Fixes: 184c39d16568 ("net/dpaa2: add DPRTC sub-module") >>>> Cc:stable@dpdk.org >>>> >>>> Signed-off-by: Chenxu Di<chenxux.di@intel.com> >>>> Signed-off-by: Ferruh Yigit<ferruh.yigit@intel.com> > > Acked-by: Sachin Saxena<sachin.saxena@oss.nxp.com> > Applied to dpdk-next-net/main, thanks. ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-10-09 11:25 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-15 2:40 [dpdk-dev] [PATCH] net/dpaa2: fix build error about timesync functions Chenxu Di 2020-09-15 3:22 ` Hemant Agrawal 2020-09-16 5:12 ` Zhou, JunX W 2020-09-16 15:29 ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit 2020-09-17 2:03 ` Di, ChenxuX 2020-09-17 11:38 ` Ferruh Yigit 2020-09-17 11:50 ` Hemant Agrawal 2020-09-17 11:59 ` Ferruh Yigit 2020-09-17 15:40 ` Hemant Agrawal 2020-09-30 15:09 ` Ferruh Yigit 2020-09-17 8:43 ` [dpdk-dev] [PATCH v2] " Chenxu Di 2020-09-17 11:48 ` Hemant Agrawal 2020-10-06 17:16 ` [dpdk-dev] [PATCH v3] " Ferruh Yigit 2020-10-08 2:24 ` Sachin Saxena (OSS) 2020-10-08 13:12 ` Ferruh Yigit 2020-10-09 7:48 ` Sachin Saxena (OSS) 2020-10-09 11:22 ` Ferruh Yigit
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).