DPDK patches and discussions
 help / color / Atom feed
* [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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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
  3 siblings, 1 reply; 11+ 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] 11+ 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; 11+ 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] 11+ 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
  0 siblings, 0 replies; 11+ 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] 11+ 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; 11+ 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] 11+ 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; 11+ 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] 11+ 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
  0 siblings, 0 replies; 11+ 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] 11+ messages in thread

end of thread, back to index

Thread overview: 11+ 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-17  8:43 ` [dpdk-dev] [PATCH v2] " Chenxu Di
2020-09-17 11:48   ` Hemant Agrawal

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox