[dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse

[dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse

[Dharmik Thakkar]

Rename 'machine' to 'machine_properties' in config/arm/meson.build since
'machine' is previously being used in config/meson.build

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 config/arm/meson.build | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/config/arm/meson.build b/config/arm/meson.build
index 8728051d5e38..e89ecdc4ccd2 100644
--- a/config/arm/meson.build
+++ b/config/arm/meson.build
@@ -141,12 +141,12 @@ else
 	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
 	dpdk_conf.set('RTE_ARCH_ARM64', 1)
 
-	machine = []
+	machine_properties = []
 	cmd_generic = ['generic', '', '', 'default', '']
 	cmd_output = cmd_generic # Set generic by default
 	machine_args = [] # Clear previous machine args
 	if arm_force_default_march and not meson.is_cross_build()
-		machine = impl_generic
+		machine_properties = impl_generic
 		impl_pn = 'default'
 	elif not meson.is_cross_build()
 		# The script returns ['Implementer', 'Variant', 'Architecture',
@@ -158,9 +158,9 @@ else
 			cmd_output = cmd.stdout().to_lower().strip().split(' ')
 		endif
 		# Set to generic if variable is not found
-		machine = get_variable('impl_' + cmd_output[0], ['generic'])
-		if machine[0] == 'generic'
-			machine = impl_generic
+		machine_properties = get_variable('impl_' + cmd_output[0], ['generic'])
+		if machine_properties[0] == 'generic'
+			machine_properties = impl_generic
 			cmd_output = cmd_generic
 		endif
 		impl_pn = cmd_output[3]
@@ -170,7 +170,7 @@ else
 	else
 		impl_id = meson.get_cross_property('implementor_id', 'generic')
 		impl_pn = meson.get_cross_property('implementor_pn', 'default')
-		machine = get_variable('impl_' + impl_id)
+		machine_properties = get_variable('impl_' + impl_id)
 	endif
 
 	# Apply Common Defaults. These settings may be overwritten by machine
@@ -181,14 +181,14 @@ else
 		endif
 	endforeach
 
-	message('Implementer : ' + machine[0])
-	foreach flag: machine[1]
+	message('Implementer : ' + machine_properties[0])
+	foreach flag: machine_properties[1]
 		if flag.length() > 0
 			dpdk_conf.set(flag[0], flag[1])
 		endif
 	endforeach
 
-	foreach marg: machine[2]
+	foreach marg: machine_properties[2]
 		if marg[0] == impl_pn
 			foreach flag: marg[1]
 				if cc.has_argument(flag)
-- 
2.17.1

[dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Dharmik Thakkar]

For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to incorrect
RTE_MAX_LCORE when machines have same Implemener and part number
but different number of CPUs.
For x86, RTE_MAX_LCORE is always set to 128 (using the value
set in meson_options.txt)

Use python script to find max lcore when using native build to
correctly set RTE_MAX_LCORE.

Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
---
 config/get_max_lcores.py | 13 +++++++++++++
 config/meson.build       | 13 ++++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)
 create mode 100755 config/get_max_lcores.py

diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py
new file mode 100755
index 000000000000..ebf1c7efdadd
--- /dev/null
+++ b/config/get_max_lcores.py
@@ -0,0 +1,13 @@
+#!/usr/bin/python3
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2020 Arm Limited
+
+import os
+
+max_lcores = []
+
+nCPU = os.cpu_count()
+
+max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
+
+print(' '.join(max_lcores))
diff --git a/config/meson.build b/config/meson.build
index 6996e5cbeaa5..80c05bc15d2f 100644
--- a/config/meson.build
+++ b/config/meson.build
@@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved memory areas
 	dpdk_conf.set('RTE_MAX_MEM_MB', 2048)
 endif
 
-
 compile_time_cpuflags = []
 subdir(arch_subdir)
 dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags))
 
+# set max lcores
+if machine != 'default' and not meson.is_cross_build()
+	# The script returns max lcores
+	params = files('get_max_lcores.py')
+	cmd_out = run_command(params)
+	if cmd_out.returncode() == 0
+		cmd_lcore = cmd_out.stdout().to_lower().strip().split(' ')
+	endif
+	max_lcore = cmd_lcore[0].to_int()
+	dpdk_conf.set('RTE_MAX_LCORE', max_lcore)
+endif
+
 # set the install path for the drivers
 dpdk_conf.set_quoted('RTE_EAL_PMD_PATH', eal_pmd_path)
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Jerin Jacob]

On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar <dharmik.thakkar@arm.com> wrote:
>
> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to incorrect
> RTE_MAX_LCORE when machines have same Implemener and part number
> but different number of CPUs.
> For x86, RTE_MAX_LCORE is always set to 128 (using the value
> set in meson_options.txt)
>
> Use python script to find max lcore when using native build to
> correctly set RTE_MAX_LCORE.

We may need to build on the native arm64 machine and use it on another
arm64 machine(Just like x86).
So I think, at least for default config(which will be used by
distribution) to support max
lcores as fixed. I am not sure this patch changes those aspects or
not? Please check.

>
> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  config/get_max_lcores.py | 13 +++++++++++++
>  config/meson.build       | 13 ++++++++++++-
>  2 files changed, 25 insertions(+), 1 deletion(-)
>  create mode 100755 config/get_max_lcores.py
>
> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py
> new file mode 100755
> index 000000000000..ebf1c7efdadd
> --- /dev/null
> +++ b/config/get_max_lcores.py
> @@ -0,0 +1,13 @@
> +#!/usr/bin/python3
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2020 Arm Limited
> +
> +import os
> +
> +max_lcores = []
> +
> +nCPU = os.cpu_count()
> +
> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
> +
> +print(' '.join(max_lcores))
> diff --git a/config/meson.build b/config/meson.build
> index 6996e5cbeaa5..80c05bc15d2f 100644
> --- a/config/meson.build
> +++ b/config/meson.build
> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved memory areas
>         dpdk_conf.set('RTE_MAX_MEM_MB', 2048)
>  endif
>
> -
>  compile_time_cpuflags = []
>  subdir(arch_subdir)
>  dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags))
>
> +# set max lcores
> +if machine != 'default' and not meson.is_cross_build()
> +       # The script returns max lcores
> +       params = files('get_max_lcores.py')
> +       cmd_out = run_command(params)
> +       if cmd_out.returncode() == 0
> +               cmd_lcore = cmd_out.stdout().to_lower().strip().split(' ')
> +       endif
> +       max_lcore = cmd_lcore[0].to_int()
> +       dpdk_conf.set('RTE_MAX_LCORE', max_lcore)
> +endif
> +
>  # set the install path for the drivers
>  dpdk_conf.set_quoted('RTE_EAL_PMD_PATH', eal_pmd_path)
>
> --
> 2.17.1
>

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Dharmik Thakkar]

> On Aug 25, 2020, at 11:47 PM, Jerin Jacob <jerinjacobk@gmail.com> wrote:
> 
> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar <dharmik.thakkar@arm.com> wrote:
>> 
>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to incorrect
>> RTE_MAX_LCORE when machines have same Implemener and part number
>> but different number of CPUs.
>> For x86, RTE_MAX_LCORE is always set to 128 (using the value
>> set in meson_options.txt)
>> 
>> Use python script to find max lcore when using native build to
>> correctly set RTE_MAX_LCORE.
> 
> We may need to build on the native arm64 machine and use it on another
> arm64 machine(Just like x86).
> So I think, at least for default config(which will be used by
> distribution) to support max
> lcores as fixed. I am not sure this patch changes those aspects or
> not? Please check.

This patch does *not* affect ‘default’ build type and cross-compilation.

> 
>> 
>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>> ---
>> config/get_max_lcores.py | 13 +++++++++++++
>> config/meson.build       | 13 ++++++++++++-
>> 2 files changed, 25 insertions(+), 1 deletion(-)
>> create mode 100755 config/get_max_lcores.py
>> 
>> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py
>> new file mode 100755
>> index 000000000000..ebf1c7efdadd
>> --- /dev/null
>> +++ b/config/get_max_lcores.py
>> @@ -0,0 +1,13 @@
>> +#!/usr/bin/python3
>> +# SPDX-License-Identifier: BSD-3-Clause
>> +# Copyright(c) 2020 Arm Limited
>> +
>> +import os
>> +
>> +max_lcores = []
>> +
>> +nCPU = os.cpu_count()
>> +
>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
>> +
>> +print(' '.join(max_lcores))
>> diff --git a/config/meson.build b/config/meson.build
>> index 6996e5cbeaa5..80c05bc15d2f 100644
>> --- a/config/meson.build
>> +++ b/config/meson.build
>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved memory areas
>>        dpdk_conf.set('RTE_MAX_MEM_MB', 2048)
>> endif
>> 
>> -
>> compile_time_cpuflags = []
>> subdir(arch_subdir)
>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS', ','.join(compile_time_cpuflags))
>> 
>> +# set max lcores
>> +if machine != 'default' and not meson.is_cross_build()
>> +       # The script returns max lcores
>> +       params = files('get_max_lcores.py')
>> +       cmd_out = run_command(params)
>> +       if cmd_out.returncode() == 0
>> +               cmd_lcore = cmd_out.stdout().to_lower().strip().split(' ')
>> +       endif
>> +       max_lcore = cmd_lcore[0].to_int()
>> +       dpdk_conf.set('RTE_MAX_LCORE', max_lcore)
>> +endif
>> +
>> # set the install path for the drivers
>> dpdk_conf.set_quoted('RTE_EAL_PMD_PATH', eal_pmd_path)
>> 
>> --
>> 2.17.1
>> 

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Juraj Linkeš]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> Sent: Wednesday, August 26, 2020 6:56 AM
> To: Jerin Jacob <jerinjacobk@gmail.com>
> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
> 
> 
> 
> > On Aug 25, 2020, at 11:47 PM, Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
> <dharmik.thakkar@arm.com> wrote:
> >>
> >> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to
> >> incorrect RTE_MAX_LCORE when machines have same Implemener and part
> >> number but different number of CPUs.
> >> For x86, RTE_MAX_LCORE is always set to 128 (using the value set in
> >> meson_options.txt)
> >>
> >> Use python script to find max lcore when using native build to
> >> correctly set RTE_MAX_LCORE.
> >
> > We may need to build on the native arm64 machine and use it on another
> > arm64 machine(Just like x86).
> > So I think, at least for default config(which will be used by
> > distribution) to support max
> > lcores as fixed. I am not sure this patch changes those aspects or
> > not? Please check.
> 
> This patch does *not* affect ‘default’ build type and cross-compilation.
> 
> >
> >>
> >> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> >> ---
> >> config/get_max_lcores.py | 13 +++++++++++++
> >> config/meson.build       | 13 ++++++++++++-
> >> 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100755
> >> config/get_max_lcores.py
> >>
> >> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py new
> >> file mode 100755 index 000000000000..ebf1c7efdadd
> >> --- /dev/null
> >> +++ b/config/get_max_lcores.py
> >> @@ -0,0 +1,13 @@
> >> +#!/usr/bin/python3
> >> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Arm
> >> +Limited
> >> +
> >> +import os
> >> +
> >> +max_lcores = []
> >> +
> >> +nCPU = os.cpu_count()
> >> +
> >> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
> >> +
> >> +print(' '.join(max_lcores))
> >> diff --git a/config/meson.build b/config/meson.build index
> >> 6996e5cbeaa5..80c05bc15d2f 100644
> >> --- a/config/meson.build
> >> +++ b/config/meson.build
> >> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved memory
> areas
> >>        dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
> >>
> >> -
> >> compile_time_cpuflags = []
> >> subdir(arch_subdir)
> >> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
> >> ','.join(compile_time_cpuflags))
> >>
> >> +# set max lcores
> >> +if machine != 'default' and not meson.is_cross_build()
> >> +       # The script returns max lcores
> >> +       params = files('get_max_lcores.py')
> >> +       cmd_out = run_command(params)

Have you considered running just a shell command, such as "nproc --all"?

> >> +       if cmd_out.returncode() == 0
> >> +               cmd_lcore = cmd_out.stdout().to_lower().strip().split(' ')
> >> +       endif
> >> +       max_lcore = cmd_lcore[0].to_int()
> >> +       dpdk_conf.set('RTE_MAX_LCORE', max_lcore) endif
> >> +
> >> # set the install path for the drivers
> >> dpdk_conf.set_quoted('RTE_EAL_PMD_PATH', eal_pmd_path)
> >>
> >> --
> >> 2.17.1
> >>

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Stephen Hemminger]

On Thu, 3 Sep 2020 06:20:17 +0000
Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:

> > -----Original Message-----
> > From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> > Sent: Wednesday, August 26, 2020 6:56 AM
> > To: Jerin Jacob <jerinjacobk@gmail.com>
> > Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd <nd@arm.com>
> > Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
> > 
> > 
> >   
> > > On Aug 25, 2020, at 11:47 PM, Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > >
> > > On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar  
> > <dharmik.thakkar@arm.com> wrote:  
> > >>
> > >> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to
> > >> incorrect RTE_MAX_LCORE when machines have same Implemener and part
> > >> number but different number of CPUs.
> > >> For x86, RTE_MAX_LCORE is always set to 128 (using the value set in
> > >> meson_options.txt)
> > >>
> > >> Use python script to find max lcore when using native build to
> > >> correctly set RTE_MAX_LCORE.  
> > >
> > > We may need to build on the native arm64 machine and use it on another
> > > arm64 machine(Just like x86).
> > > So I think, at least for default config(which will be used by
> > > distribution) to support max
> > > lcores as fixed. I am not sure this patch changes those aspects or
> > > not? Please check.  
> > 
> > This patch does *not* affect ‘default’ build type and cross-compilation.
> >   
> > >  
> > >>
> > >> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > >> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > >> ---
> > >> config/get_max_lcores.py | 13 +++++++++++++
> > >> config/meson.build       | 13 ++++++++++++-
> > >> 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100755
> > >> config/get_max_lcores.py
> > >>
> > >> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py new
> > >> file mode 100755 index 000000000000..ebf1c7efdadd
> > >> --- /dev/null
> > >> +++ b/config/get_max_lcores.py
> > >> @@ -0,0 +1,13 @@
> > >> +#!/usr/bin/python3
> > >> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Arm
> > >> +Limited
> > >> +
> > >> +import os
> > >> +
> > >> +max_lcores = []
> > >> +
> > >> +nCPU = os.cpu_count()
> > >> +
> > >> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
> > >> +
> > >> +print(' '.join(max_lcores))
> > >> diff --git a/config/meson.build b/config/meson.build index
> > >> 6996e5cbeaa5..80c05bc15d2f 100644
> > >> --- a/config/meson.build
> > >> +++ b/config/meson.build
> > >> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved memory  
> > areas  
> > >>        dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
> > >>
> > >> -
> > >> compile_time_cpuflags = []
> > >> subdir(arch_subdir)
> > >> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
> > >> ','.join(compile_time_cpuflags))
> > >>
> > >> +# set max lcores
> > >> +if machine != 'default' and not meson.is_cross_build()
> > >> +       # The script returns max lcores
> > >> +       params = files('get_max_lcores.py')
> > >> +       cmd_out = run_command(params)  
> 
> Have you considered running just a shell command, such as "nproc --all"?

Is this really a good idea?
For real distributions and NFV products, the build and runtime environment will usually be
different even if on same CPU architecture.

In many cases there maybe a huge build machine (128 CPU) or in a container (reported as single cpu)
even if not doing cross build.

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Dharmik Thakkar]

> On Sep 3, 2020, at 5:52 PM, Stephen Hemminger <stephen@networkplumber.org> wrote:
> 
> On Thu, 3 Sep 2020 06:20:17 +0000
> Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> 
>>> -----Original Message-----
>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>>> Sent: Wednesday, August 26, 2020 6:56 AM
>>> To: Jerin Jacob <jerinjacobk@gmail.com>
>>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd <nd@arm.com>
>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
>>> 
>>> 
>>> 
>>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob <jerinjacobk@gmail.com> wrote:
>>>> 
>>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar  
>>> <dharmik.thakkar@arm.com> wrote:  
>>>>> 
>>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to
>>>>> incorrect RTE_MAX_LCORE when machines have same Implemener and part
>>>>> number but different number of CPUs.
>>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value set in
>>>>> meson_options.txt)
>>>>> 
>>>>> Use python script to find max lcore when using native build to
>>>>> correctly set RTE_MAX_LCORE.  
>>>> 
>>>> We may need to build on the native arm64 machine and use it on another
>>>> arm64 machine(Just like x86).
>>>> So I think, at least for default config(which will be used by
>>>> distribution) to support max
>>>> lcores as fixed. I am not sure this patch changes those aspects or
>>>> not? Please check.  
>>> 
>>> This patch does *not* affect ‘default’ build type and cross-compilation.
>>> 
>>>> 
>>>>> 
>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>>>>> ---
>>>>> config/get_max_lcores.py | 13 +++++++++++++
>>>>> config/meson.build       | 13 ++++++++++++-
>>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100755
>>>>> config/get_max_lcores.py
>>>>> 
>>>>> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py new
>>>>> file mode 100755 index 000000000000..ebf1c7efdadd
>>>>> --- /dev/null
>>>>> +++ b/config/get_max_lcores.py
>>>>> @@ -0,0 +1,13 @@
>>>>> +#!/usr/bin/python3
>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Arm
>>>>> +Limited
>>>>> +
>>>>> +import os
>>>>> +
>>>>> +max_lcores = []
>>>>> +
>>>>> +nCPU = os.cpu_count()
>>>>> +
>>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
>>>>> +
>>>>> +print(' '.join(max_lcores))
>>>>> diff --git a/config/meson.build b/config/meson.build index
>>>>> 6996e5cbeaa5..80c05bc15d2f 100644
>>>>> --- a/config/meson.build
>>>>> +++ b/config/meson.build
>>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved memory  
>>> areas  
>>>>>       dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
>>>>> 
>>>>> -
>>>>> compile_time_cpuflags = []
>>>>> subdir(arch_subdir)
>>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
>>>>> ','.join(compile_time_cpuflags))
>>>>> 
>>>>> +# set max lcores
>>>>> +if machine != 'default' and not meson.is_cross_build()
>>>>> +       # The script returns max lcores
>>>>> +       params = files('get_max_lcores.py')
>>>>> +       cmd_out = run_command(params)  
>> 
>> Have you considered running just a shell command, such as "nproc --all"?
> 
> Is this really a good idea?
> For real distributions and NFV products, the build and runtime environment will usually be
> different even if on same CPU architecture.
> 
> In many cases there maybe a huge build machine (128 CPU) or in a container (reported as single cpu)
> even if not doing cross build.

That’s a great point, Stephen. IMO, this patch is useful when building and running natively.
For all other purposes (like the ones you mentioned), do you think it is a good idea to set RTE_MAX_LCORE using -Dmax_lcores?

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Juraj Linkeš]

> -----Original Message-----
> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> Sent: Friday, September 4, 2020 7:44 AM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
> <jerinjacobk@gmail.com>; thomas@monjalon.net; dpdk-dev <dev@dpdk.org>;
> nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
> 
> 
> 
> > On Sep 3, 2020, at 5:52 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Thu, 3 Sep 2020 06:20:17 +0000
> > Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> >
> >>> -----Original Message-----
> >>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> >>> Sent: Wednesday, August 26, 2020 6:56 AM
> >>> To: Jerin Jacob <jerinjacobk@gmail.com>
> >>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd <nd@arm.com>
> >>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
> >>> programmatically
> >>>
> >>>
> >>>
> >>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob <jerinjacobk@gmail.com>
> wrote:
> >>>>
> >>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
> >>> <dharmik.thakkar@arm.com> wrote:
> >>>>>
> >>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to
> >>>>> incorrect RTE_MAX_LCORE when machines have same Implemener and
> >>>>> part number but different number of CPUs.
> >>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value set
> >>>>> in
> >>>>> meson_options.txt)
> >>>>>
> >>>>> Use python script to find max lcore when using native build to
> >>>>> correctly set RTE_MAX_LCORE.
> >>>>
> >>>> We may need to build on the native arm64 machine and use it on
> >>>> another
> >>>> arm64 machine(Just like x86).
> >>>> So I think, at least for default config(which will be used by
> >>>> distribution) to support max
> >>>> lcores as fixed. I am not sure this patch changes those aspects or
> >>>> not? Please check.
> >>>
> >>> This patch does *not* affect ‘default’ build type and cross-compilation.
> >>>
> >>>>
> >>>>>
> >>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> >>>>> ---
> >>>>> config/get_max_lcores.py | 13 +++++++++++++
> >>>>> config/meson.build       | 13 ++++++++++++-
> >>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode
> >>>>> 100755 config/get_max_lcores.py
> >>>>>
> >>>>> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py
> >>>>> new file mode 100755 index 000000000000..ebf1c7efdadd
> >>>>> --- /dev/null
> >>>>> +++ b/config/get_max_lcores.py
> >>>>> @@ -0,0 +1,13 @@
> >>>>> +#!/usr/bin/python3
> >>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Arm
> >>>>> +Limited
> >>>>> +
> >>>>> +import os
> >>>>> +
> >>>>> +max_lcores = []
> >>>>> +
> >>>>> +nCPU = os.cpu_count()
> >>>>> +
> >>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
> >>>>> +
> >>>>> +print(' '.join(max_lcores))
> >>>>> diff --git a/config/meson.build b/config/meson.build index
> >>>>> 6996e5cbeaa5..80c05bc15d2f 100644
> >>>>> --- a/config/meson.build
> >>>>> +++ b/config/meson.build
> >>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved
> >>>>> memory
> >>> areas
> >>>>>       dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
> >>>>>
> >>>>> -
> >>>>> compile_time_cpuflags = []
> >>>>> subdir(arch_subdir)
> >>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
> >>>>> ','.join(compile_time_cpuflags))
> >>>>>
> >>>>> +# set max lcores
> >>>>> +if machine != 'default' and not meson.is_cross_build()
> >>>>> +       # The script returns max lcores
> >>>>> +       params = files('get_max_lcores.py')
> >>>>> +       cmd_out = run_command(params)
> >>
> >> Have you considered running just a shell command, such as "nproc --all"?
> >
> > Is this really a good idea?
> > For real distributions and NFV products, the build and runtime
> > environment will usually be different even if on same CPU architecture.
> >
> > In many cases there maybe a huge build machine (128 CPU) or in a
> > container (reported as single cpu) even if not doing cross build.
> 
> That’s a great point, Stephen. IMO, this patch is useful when building and
> running natively.
> For all other purposes (like the ones you mentioned), do you think it is a good
> idea to set RTE_MAX_LCORE using -Dmax_lcores?

We should only use this native builds, as that would be consistent with the current meson build philosophy of "meson figuring as much as possible on its own". Any build other than native implies the user wants to deviate from the build machine.

One way to do this automatic core count is when max_lcores=0 (0 would have the special meaning of "figure core count automatically"). We can set that as default in meson_option.txt and then users will have the ability to set it to whatever they want, even for native builds. What do you think?

Currently the -Dmax_lcores option doesn't work for arm builds (the value of RTE_MAX_LCORE is overwritten in config/arm/meson.build). I believe the patch tries to address this, but still, we need to be mindful of that.

Juraj

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Dharmik Thakkar]

> On Sep 17, 2020, at 4:56 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
>> Sent: Friday, September 4, 2020 7:44 AM
>> To: Stephen Hemminger <stephen@networkplumber.org>
>> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
>> <jerinjacobk@gmail.com>; thomas@monjalon.net; dpdk-dev <dev@dpdk.org>;
>> nd <nd@arm.com>
>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
>> 
>> 
>> 
>>> On Sep 3, 2020, at 5:52 PM, Stephen Hemminger
>> <stephen@networkplumber.org> wrote:
>>> 
>>> On Thu, 3 Sep 2020 06:20:17 +0000
>>> Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>> 
>>>>> -----Original Message-----
>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>>>>> Sent: Wednesday, August 26, 2020 6:56 AM
>>>>> To: Jerin Jacob <jerinjacobk@gmail.com>
>>>>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd <nd@arm.com>
>>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
>>>>> programmatically
>>>>> 
>>>>> 
>>>>> 
>>>>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob <jerinjacobk@gmail.com>
>> wrote:
>>>>>> 
>>>>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
>>>>> <dharmik.thakkar@arm.com> wrote:
>>>>>>> 
>>>>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to
>>>>>>> incorrect RTE_MAX_LCORE when machines have same Implemener and
>>>>>>> part number but different number of CPUs.
>>>>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value set
>>>>>>> in
>>>>>>> meson_options.txt)
>>>>>>> 
>>>>>>> Use python script to find max lcore when using native build to
>>>>>>> correctly set RTE_MAX_LCORE.
>>>>>> 
>>>>>> We may need to build on the native arm64 machine and use it on
>>>>>> another
>>>>>> arm64 machine(Just like x86).
>>>>>> So I think, at least for default config(which will be used by
>>>>>> distribution) to support max
>>>>>> lcores as fixed. I am not sure this patch changes those aspects or
>>>>>> not? Please check.
>>>>> 
>>>>> This patch does *not* affect ‘default’ build type and cross-compilation.
>>>>> 
>>>>>> 
>>>>>>> 
>>>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>>>>>>> ---
>>>>>>> config/get_max_lcores.py | 13 +++++++++++++
>>>>>>> config/meson.build       | 13 ++++++++++++-
>>>>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode
>>>>>>> 100755 config/get_max_lcores.py
>>>>>>> 
>>>>>>> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py
>>>>>>> new file mode 100755 index 000000000000..ebf1c7efdadd
>>>>>>> --- /dev/null
>>>>>>> +++ b/config/get_max_lcores.py
>>>>>>> @@ -0,0 +1,13 @@
>>>>>>> +#!/usr/bin/python3
>>>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Arm
>>>>>>> +Limited
>>>>>>> +
>>>>>>> +import os
>>>>>>> +
>>>>>>> +max_lcores = []
>>>>>>> +
>>>>>>> +nCPU = os.cpu_count()
>>>>>>> +
>>>>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
>>>>>>> +
>>>>>>> +print(' '.join(max_lcores))
>>>>>>> diff --git a/config/meson.build b/config/meson.build index
>>>>>>> 6996e5cbeaa5..80c05bc15d2f 100644
>>>>>>> --- a/config/meson.build
>>>>>>> +++ b/config/meson.build
>>>>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved
>>>>>>> memory
>>>>> areas
>>>>>>>      dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
>>>>>>> 
>>>>>>> -
>>>>>>> compile_time_cpuflags = []
>>>>>>> subdir(arch_subdir)
>>>>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
>>>>>>> ','.join(compile_time_cpuflags))
>>>>>>> 
>>>>>>> +# set max lcores
>>>>>>> +if machine != 'default' and not meson.is_cross_build()
>>>>>>> +       # The script returns max lcores
>>>>>>> +       params = files('get_max_lcores.py')
>>>>>>> +       cmd_out = run_command(params)
>>>> 
>>>> Have you considered running just a shell command, such as "nproc --all"?
>>> 
>>> Is this really a good idea?
>>> For real distributions and NFV products, the build and runtime
>>> environment will usually be different even if on same CPU architecture.
>>> 
>>> In many cases there maybe a huge build machine (128 CPU) or in a
>>> container (reported as single cpu) even if not doing cross build.
>> 
>> That’s a great point, Stephen. IMO, this patch is useful when building and
>> running natively.
>> For all other purposes (like the ones you mentioned), do you think it is a good
>> idea to set RTE_MAX_LCORE using -Dmax_lcores?
> 
> We should only use this native builds, as that would be consistent with the current meson build philosophy of "meson figuring as much as possible on its own". Any build other than native implies the user wants to deviate from the build machine.
> 

The MIDR value-based probing doesn’t quite work well for Arm IP (currently being discussed on this patch: https://patches.dpdk.org/patch/76981/).

> One way to do this automatic core count is when max_lcores=0 (0 would have the special meaning of "figure core count automatically"). We can set that as default in meson_option.txt and then users will have the ability to set it to whatever they want, even for native builds. What do you think?
> 

Yes, agreed.

> Currently the -Dmax_lcores option doesn't work for arm builds (the value of RTE_MAX_LCORE is overwritten in config/arm/meson.build). I believe the patch tries to address this, but still, we need to be mindful of that.
> 
> Juraj

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Thomas Monjalon]

Please, what is the conclusion here?


18/09/2020 07:47, Dharmik Thakkar:
> 
> > On Sep 17, 2020, at 4:56 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> > 
> > 
> > 
> >> -----Original Message-----
> >> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> >> Sent: Friday, September 4, 2020 7:44 AM
> >> To: Stephen Hemminger <stephen@networkplumber.org>
> >> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
> >> <jerinjacobk@gmail.com>; thomas@monjalon.net; dpdk-dev <dev@dpdk.org>;
> >> nd <nd@arm.com>
> >> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
> >> 
> >> 
> >> 
> >>> On Sep 3, 2020, at 5:52 PM, Stephen Hemminger
> >> <stephen@networkplumber.org> wrote:
> >>> 
> >>> On Thu, 3 Sep 2020 06:20:17 +0000
> >>> Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> >>> 
> >>>>> -----Original Message-----
> >>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> >>>>> Sent: Wednesday, August 26, 2020 6:56 AM
> >>>>> To: Jerin Jacob <jerinjacobk@gmail.com>
> >>>>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd <nd@arm.com>
> >>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
> >>>>> programmatically
> >>>>> 
> >>>>> 
> >>>>> 
> >>>>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob <jerinjacobk@gmail.com>
> >> wrote:
> >>>>>> 
> >>>>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
> >>>>> <dharmik.thakkar@arm.com> wrote:
> >>>>>>> 
> >>>>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to
> >>>>>>> incorrect RTE_MAX_LCORE when machines have same Implemener and
> >>>>>>> part number but different number of CPUs.
> >>>>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value set
> >>>>>>> in
> >>>>>>> meson_options.txt)
> >>>>>>> 
> >>>>>>> Use python script to find max lcore when using native build to
> >>>>>>> correctly set RTE_MAX_LCORE.
> >>>>>> 
> >>>>>> We may need to build on the native arm64 machine and use it on
> >>>>>> another
> >>>>>> arm64 machine(Just like x86).
> >>>>>> So I think, at least for default config(which will be used by
> >>>>>> distribution) to support max
> >>>>>> lcores as fixed. I am not sure this patch changes those aspects or
> >>>>>> not? Please check.
> >>>>> 
> >>>>> This patch does *not* affect ‘default’ build type and cross-compilation.
> >>>>> 
> >>>>>> 
> >>>>>>> 
> >>>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>>>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> >>>>>>> ---
> >>>>>>> config/get_max_lcores.py | 13 +++++++++++++
> >>>>>>> config/meson.build       | 13 ++++++++++++-
> >>>>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode
> >>>>>>> 100755 config/get_max_lcores.py
> >>>>>>> 
> >>>>>>> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py
> >>>>>>> new file mode 100755 index 000000000000..ebf1c7efdadd
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/config/get_max_lcores.py
> >>>>>>> @@ -0,0 +1,13 @@
> >>>>>>> +#!/usr/bin/python3
> >>>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Arm
> >>>>>>> +Limited
> >>>>>>> +
> >>>>>>> +import os
> >>>>>>> +
> >>>>>>> +max_lcores = []
> >>>>>>> +
> >>>>>>> +nCPU = os.cpu_count()
> >>>>>>> +
> >>>>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
> >>>>>>> +
> >>>>>>> +print(' '.join(max_lcores))
> >>>>>>> diff --git a/config/meson.build b/config/meson.build index
> >>>>>>> 6996e5cbeaa5..80c05bc15d2f 100644
> >>>>>>> --- a/config/meson.build
> >>>>>>> +++ b/config/meson.build
> >>>>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved
> >>>>>>> memory
> >>>>> areas
> >>>>>>>      dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
> >>>>>>> 
> >>>>>>> -
> >>>>>>> compile_time_cpuflags = []
> >>>>>>> subdir(arch_subdir)
> >>>>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
> >>>>>>> ','.join(compile_time_cpuflags))
> >>>>>>> 
> >>>>>>> +# set max lcores
> >>>>>>> +if machine != 'default' and not meson.is_cross_build()
> >>>>>>> +       # The script returns max lcores
> >>>>>>> +       params = files('get_max_lcores.py')
> >>>>>>> +       cmd_out = run_command(params)
> >>>> 
> >>>> Have you considered running just a shell command, such as "nproc --all"?
> >>> 
> >>> Is this really a good idea?
> >>> For real distributions and NFV products, the build and runtime
> >>> environment will usually be different even if on same CPU architecture.
> >>> 
> >>> In many cases there maybe a huge build machine (128 CPU) or in a
> >>> container (reported as single cpu) even if not doing cross build.
> >> 
> >> That’s a great point, Stephen. IMO, this patch is useful when building and
> >> running natively.
> >> For all other purposes (like the ones you mentioned), do you think it is a good
> >> idea to set RTE_MAX_LCORE using -Dmax_lcores?
> > 
> > We should only use this native builds, as that would be consistent with the current meson build philosophy of "meson figuring as much as possible on its own". Any build other than native implies the user wants to deviate from the build machine.
> > 
> 
> The MIDR value-based probing doesn’t quite work well for Arm IP (currently being discussed on this patch: https://patches.dpdk.org/patch/76981/).
> 
> > One way to do this automatic core count is when max_lcores=0 (0 would have the special meaning of "figure core count automatically"). We can set that as default in meson_option.txt and then users will have the ability to set it to whatever they want, even for native builds. What do you think?
> > 
> 
> Yes, agreed.
> 
> > Currently the -Dmax_lcores option doesn't work for arm builds (the value of RTE_MAX_LCORE is overwritten in config/arm/meson.build). I believe the patch tries to address this, but still, we need to be mindful of that.
> > 
> > Juraj

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Juraj Linkeš]

I believe we're going to drop this patch series in favor of http://patches.dpdk.org/project/dpdk/list/?series=12923. 

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Tuesday, October 13, 2020 4:32 PM
> To: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; dev@dpdk.org; Stephen
> Hemminger <stephen@networkplumber.org>; Jerin Jacob
> <jerinjacobk@gmail.com>; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
> 
> Please, what is the conclusion here?
> 
> 
> 18/09/2020 07:47, Dharmik Thakkar:
> >
> > > On Sep 17, 2020, at 4:56 AM, Juraj Linkeš <juraj.linkes@pantheon.tech>
> wrote:
> > >
> > >
> > >
> > >> -----Original Message-----
> > >> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> > >> Sent: Friday, September 4, 2020 7:44 AM
> > >> To: Stephen Hemminger <stephen@networkplumber.org>
> > >> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
> > >> <jerinjacobk@gmail.com>; thomas@monjalon.net; dpdk-dev
> > >> <dev@dpdk.org>; nd <nd@arm.com>
> > >> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
> > >> programmatically
> > >>
> > >>
> > >>
> > >>> On Sep 3, 2020, at 5:52 PM, Stephen Hemminger
> > >> <stephen@networkplumber.org> wrote:
> > >>>
> > >>> On Thu, 3 Sep 2020 06:20:17 +0000
> > >>> Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> > >>>
> > >>>>> -----Original Message-----
> > >>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> > >>>>> Sent: Wednesday, August 26, 2020 6:56 AM
> > >>>>> To: Jerin Jacob <jerinjacobk@gmail.com>
> > >>>>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd
> > >>>>> <nd@arm.com>
> > >>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
> > >>>>> programmatically
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob
> > >>>>>> <jerinjacobk@gmail.com>
> > >> wrote:
> > >>>>>>
> > >>>>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
> > >>>>> <dharmik.thakkar@arm.com> wrote:
> > >>>>>>>
> > >>>>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads
> > >>>>>>> to incorrect RTE_MAX_LCORE when machines have same
> Implemener
> > >>>>>>> and part number but different number of CPUs.
> > >>>>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value
> > >>>>>>> set in
> > >>>>>>> meson_options.txt)
> > >>>>>>>
> > >>>>>>> Use python script to find max lcore when using native build to
> > >>>>>>> correctly set RTE_MAX_LCORE.
> > >>>>>>
> > >>>>>> We may need to build on the native arm64 machine and use it on
> > >>>>>> another
> > >>>>>> arm64 machine(Just like x86).
> > >>>>>> So I think, at least for default config(which will be used by
> > >>>>>> distribution) to support max
> > >>>>>> lcores as fixed. I am not sure this patch changes those aspects
> > >>>>>> or not? Please check.
> > >>>>>
> > >>>>> This patch does *not* affect ‘default’ build type and cross-compilation.
> > >>>>>
> > >>>>>>
> > >>>>>>>
> > >>>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> > >>>>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > >>>>>>> ---
> > >>>>>>> config/get_max_lcores.py | 13 +++++++++++++
> > >>>>>>> config/meson.build       | 13 ++++++++++++-
> > >>>>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode
> > >>>>>>> 100755 config/get_max_lcores.py
> > >>>>>>>
> > >>>>>>> diff --git a/config/get_max_lcores.py
> > >>>>>>> b/config/get_max_lcores.py new file mode 100755 index
> > >>>>>>> 000000000000..ebf1c7efdadd
> > >>>>>>> --- /dev/null
> > >>>>>>> +++ b/config/get_max_lcores.py
> > >>>>>>> @@ -0,0 +1,13 @@
> > >>>>>>> +#!/usr/bin/python3
> > >>>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020
> > >>>>>>> +Arm Limited
> > >>>>>>> +
> > >>>>>>> +import os
> > >>>>>>> +
> > >>>>>>> +max_lcores = []
> > >>>>>>> +
> > >>>>>>> +nCPU = os.cpu_count()
> > >>>>>>> +
> > >>>>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
> > >>>>>>> +
> > >>>>>>> +print(' '.join(max_lcores))
> > >>>>>>> diff --git a/config/meson.build b/config/meson.build index
> > >>>>>>> 6996e5cbeaa5..80c05bc15d2f 100644
> > >>>>>>> --- a/config/meson.build
> > >>>>>>> +++ b/config/meson.build
> > >>>>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller
> > >>>>>>> reserved memory
> > >>>>> areas
> > >>>>>>>      dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
> > >>>>>>>
> > >>>>>>> -
> > >>>>>>> compile_time_cpuflags = []
> > >>>>>>> subdir(arch_subdir)
> > >>>>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
> > >>>>>>> ','.join(compile_time_cpuflags))
> > >>>>>>>
> > >>>>>>> +# set max lcores
> > >>>>>>> +if machine != 'default' and not meson.is_cross_build()
> > >>>>>>> +       # The script returns max lcores
> > >>>>>>> +       params = files('get_max_lcores.py')
> > >>>>>>> +       cmd_out = run_command(params)
> > >>>>
> > >>>> Have you considered running just a shell command, such as "nproc --all"?
> > >>>
> > >>> Is this really a good idea?
> > >>> For real distributions and NFV products, the build and runtime
> > >>> environment will usually be different even if on same CPU architecture.
> > >>>
> > >>> In many cases there maybe a huge build machine (128 CPU) or in a
> > >>> container (reported as single cpu) even if not doing cross build.
> > >>
> > >> That’s a great point, Stephen. IMO, this patch is useful when
> > >> building and running natively.
> > >> For all other purposes (like the ones you mentioned), do you think
> > >> it is a good idea to set RTE_MAX_LCORE using -Dmax_lcores?
> > >
> > > We should only use this native builds, as that would be consistent with the
> current meson build philosophy of "meson figuring as much as possible on its
> own". Any build other than native implies the user wants to deviate from the
> build machine.
> > >
> >
> > The MIDR value-based probing doesn’t quite work well for Arm IP (currently
> being discussed on this patch: https://patches.dpdk.org/patch/76981/).
> >
> > > One way to do this automatic core count is when max_lcores=0 (0 would
> have the special meaning of "figure core count automatically"). We can set that
> as default in meson_option.txt and then users will have the ability to set it to
> whatever they want, even for native builds. What do you think?
> > >
> >
> > Yes, agreed.
> >
> > > Currently the -Dmax_lcores option doesn't work for arm builds (the value of
> RTE_MAX_LCORE is overwritten in config/arm/meson.build). I believe the patch
> tries to address this, but still, we need to be mindful of that.
> > >
> > > Juraj
> 
> 

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Dharmik Thakkar]

Hi Juraj,

> On Oct 13, 2020, at 9:58 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> 
> I believe we're going to drop this patch series in favor of http://patches.dpdk.org/project/dpdk/list/?series=12923.

I can see you have included this feature in your series. Thank you!
What are your thoughts on the other patch [1]? Do you plan on including that as well in your series?

[1] 	[1/2] config/arm: avoid variable reuse
https://patches.dpdk.org/patch/75946/

> 
>> -----Original Message-----
>> From: Thomas Monjalon <thomas@monjalon.net>
>> Sent: Tuesday, October 13, 2020 4:32 PM
>> To: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
>> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; dev@dpdk.org; Stephen
>> Hemminger <stephen@networkplumber.org>; Jerin Jacob
>> <jerinjacobk@gmail.com>; nd <nd@arm.com>
>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
>> 
>> Please, what is the conclusion here?
>> 
>> 
>> 18/09/2020 07:47, Dharmik Thakkar:
>>> 
>>>> On Sep 17, 2020, at 4:56 AM, Juraj Linkeš <juraj.linkes@pantheon.tech>
>> wrote:
>>>> 
>>>> 
>>>> 
>>>>> -----Original Message-----
>>>>> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
>>>>> Sent: Friday, September 4, 2020 7:44 AM
>>>>> To: Stephen Hemminger <stephen@networkplumber.org>
>>>>> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
>>>>> <jerinjacobk@gmail.com>; thomas@monjalon.net; dpdk-dev
>>>>> <dev@dpdk.org>; nd <nd@arm.com>
>>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
>>>>> programmatically
>>>>> 
>>>>> 
>>>>> 
>>>>>> On Sep 3, 2020, at 5:52 PM, Stephen Hemminger
>>>>> <stephen@networkplumber.org> wrote:
>>>>>> 
>>>>>> On Thu, 3 Sep 2020 06:20:17 +0000
>>>>>> Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>>>>> 
>>>>>>>> -----Original Message-----
>>>>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>>>>>>>> Sent: Wednesday, August 26, 2020 6:56 AM
>>>>>>>> To: Jerin Jacob <jerinjacobk@gmail.com>
>>>>>>>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd
>>>>>>>> <nd@arm.com>
>>>>>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
>>>>>>>> programmatically
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob
>>>>>>>>> <jerinjacobk@gmail.com>
>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
>>>>>>>> <dharmik.thakkar@arm.com> wrote:
>>>>>>>>>> 
>>>>>>>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads
>>>>>>>>>> to incorrect RTE_MAX_LCORE when machines have same
>> Implemener
>>>>>>>>>> and part number but different number of CPUs.
>>>>>>>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value
>>>>>>>>>> set in
>>>>>>>>>> meson_options.txt)
>>>>>>>>>> 
>>>>>>>>>> Use python script to find max lcore when using native build to
>>>>>>>>>> correctly set RTE_MAX_LCORE.
>>>>>>>>> 
>>>>>>>>> We may need to build on the native arm64 machine and use it on
>>>>>>>>> another
>>>>>>>>> arm64 machine(Just like x86).
>>>>>>>>> So I think, at least for default config(which will be used by
>>>>>>>>> distribution) to support max
>>>>>>>>> lcores as fixed. I am not sure this patch changes those aspects
>>>>>>>>> or not? Please check.
>>>>>>>> 
>>>>>>>> This patch does *not* affect ‘default’ build type and cross-compilation.
>>>>>>>> 
>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>>>>>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>>>>>>>>>> ---
>>>>>>>>>> config/get_max_lcores.py | 13 +++++++++++++
>>>>>>>>>> config/meson.build       | 13 ++++++++++++-
>>>>>>>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode
>>>>>>>>>> 100755 config/get_max_lcores.py
>>>>>>>>>> 
>>>>>>>>>> diff --git a/config/get_max_lcores.py
>>>>>>>>>> b/config/get_max_lcores.py new file mode 100755 index
>>>>>>>>>> 000000000000..ebf1c7efdadd
>>>>>>>>>> --- /dev/null
>>>>>>>>>> +++ b/config/get_max_lcores.py
>>>>>>>>>> @@ -0,0 +1,13 @@
>>>>>>>>>> +#!/usr/bin/python3
>>>>>>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020
>>>>>>>>>> +Arm Limited
>>>>>>>>>> +
>>>>>>>>>> +import os
>>>>>>>>>> +
>>>>>>>>>> +max_lcores = []
>>>>>>>>>> +
>>>>>>>>>> +nCPU = os.cpu_count()
>>>>>>>>>> +
>>>>>>>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
>>>>>>>>>> +
>>>>>>>>>> +print(' '.join(max_lcores))
>>>>>>>>>> diff --git a/config/meson.build b/config/meson.build index
>>>>>>>>>> 6996e5cbeaa5..80c05bc15d2f 100644
>>>>>>>>>> --- a/config/meson.build
>>>>>>>>>> +++ b/config/meson.build
>>>>>>>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller
>>>>>>>>>> reserved memory
>>>>>>>> areas
>>>>>>>>>>     dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
>>>>>>>>>> 
>>>>>>>>>> -
>>>>>>>>>> compile_time_cpuflags = []
>>>>>>>>>> subdir(arch_subdir)
>>>>>>>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
>>>>>>>>>> ','.join(compile_time_cpuflags))
>>>>>>>>>> 
>>>>>>>>>> +# set max lcores
>>>>>>>>>> +if machine != 'default' and not meson.is_cross_build()
>>>>>>>>>> +       # The script returns max lcores
>>>>>>>>>> +       params = files('get_max_lcores.py')
>>>>>>>>>> +       cmd_out = run_command(params)
>>>>>>> 
>>>>>>> Have you considered running just a shell command, such as "nproc --all"?
>>>>>> 
>>>>>> Is this really a good idea?
>>>>>> For real distributions and NFV products, the build and runtime
>>>>>> environment will usually be different even if on same CPU architecture.
>>>>>> 
>>>>>> In many cases there maybe a huge build machine (128 CPU) or in a
>>>>>> container (reported as single cpu) even if not doing cross build.
>>>>> 
>>>>> That’s a great point, Stephen. IMO, this patch is useful when
>>>>> building and running natively.
>>>>> For all other purposes (like the ones you mentioned), do you think
>>>>> it is a good idea to set RTE_MAX_LCORE using -Dmax_lcores?
>>>> 
>>>> We should only use this native builds, as that would be consistent with the
>> current meson build philosophy of "meson figuring as much as possible on its
>> own". Any build other than native implies the user wants to deviate from the
>> build machine.
>>>> 
>>> 
>>> The MIDR value-based probing doesn’t quite work well for Arm IP (currently
>> being discussed on this patch: https://patches.dpdk.org/patch/76981/).
>>> 
>>>> One way to do this automatic core count is when max_lcores=0 (0 would
>> have the special meaning of "figure core count automatically"). We can set that
>> as default in meson_option.txt and then users will have the ability to set it to
>> whatever they want, even for native builds. What do you think?
>>>> 
>>> 
>>> Yes, agreed.
>>> 
>>>> Currently the -Dmax_lcores option doesn't work for arm builds (the value of
>> RTE_MAX_LCORE is overwritten in config/arm/meson.build). I believe the patch
>> tries to address this, but still, we need to be mindful of that.
>>>> 
>>>> Juraj
>> 
>> 
> 

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Juraj Linkeš]

Hi Dharmik,

> -----Original Message-----
> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> Sent: Tuesday, October 13, 2020 5:15 PM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; dev@dpdk.org; Stephen Hemminger
> <stephen@networkplumber.org>; Jerin Jacob <jerinjacobk@gmail.com>; nd
> <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
> 
> Hi Juraj,
> 
> > On Oct 13, 2020, at 9:58 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> >
> > I believe we're going to drop this patch series in favor of
> http://patches.dpdk.org/project/dpdk/list/?series=12923.
> 
> I can see you have included this feature in your series. Thank you!
> What are your thoughts on the other patch [1]? Do you plan on including that as
> well in your series?
> 
> [1] 	[1/2] config/arm: avoid variable reuse
> https://patches.dpdk.org/patch/75946/
> 

I believe the general idea of your patch is alredy part of my patch series in this patch: http://patches.dpdk.org/patch/80572/

> >
> >> -----Original Message-----
> >> From: Thomas Monjalon <thomas@monjalon.net>
> >> Sent: Tuesday, October 13, 2020 4:32 PM
> >> To: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> >> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; dev@dpdk.org; Stephen
> >> Hemminger <stephen@networkplumber.org>; Jerin Jacob
> >> <jerinjacobk@gmail.com>; nd <nd@arm.com>
> >> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
> >> programmatically
> >>
> >> Please, what is the conclusion here?
> >>
> >>
> >> 18/09/2020 07:47, Dharmik Thakkar:
> >>>
> >>>> On Sep 17, 2020, at 4:56 AM, Juraj Linkeš
> >>>> <juraj.linkes@pantheon.tech>
> >> wrote:
> >>>>
> >>>>
> >>>>
> >>>>> -----Original Message-----
> >>>>> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> >>>>> Sent: Friday, September 4, 2020 7:44 AM
> >>>>> To: Stephen Hemminger <stephen@networkplumber.org>
> >>>>> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
> >>>>> <jerinjacobk@gmail.com>; thomas@monjalon.net; dpdk-dev
> >>>>> <dev@dpdk.org>; nd <nd@arm.com>
> >>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
> >>>>> programmatically
> >>>>>
> >>>>>
> >>>>>
> >>>>>> On Sep 3, 2020, at 5:52 PM, Stephen Hemminger
> >>>>> <stephen@networkplumber.org> wrote:
> >>>>>>
> >>>>>> On Thu, 3 Sep 2020 06:20:17 +0000 Juraj Linkeš
> >>>>>> <juraj.linkes@pantheon.tech> wrote:
> >>>>>>
> >>>>>>>> -----Original Message-----
> >>>>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> >>>>>>>> Sent: Wednesday, August 26, 2020 6:56 AM
> >>>>>>>> To: Jerin Jacob <jerinjacobk@gmail.com>
> >>>>>>>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd
> >>>>>>>> <nd@arm.com>
> >>>>>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
> >>>>>>>> programmatically
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob
> >>>>>>>>> <jerinjacobk@gmail.com>
> >>>>> wrote:
> >>>>>>>>>
> >>>>>>>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
> >>>>>>>> <dharmik.thakkar@arm.com> wrote:
> >>>>>>>>>>
> >>>>>>>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It
> >>>>>>>>>> leads to incorrect RTE_MAX_LCORE when machines have same
> >> Implemener
> >>>>>>>>>> and part number but different number of CPUs.
> >>>>>>>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value
> >>>>>>>>>> set in
> >>>>>>>>>> meson_options.txt)
> >>>>>>>>>>
> >>>>>>>>>> Use python script to find max lcore when using native build
> >>>>>>>>>> to correctly set RTE_MAX_LCORE.
> >>>>>>>>>
> >>>>>>>>> We may need to build on the native arm64 machine and use it on
> >>>>>>>>> another
> >>>>>>>>> arm64 machine(Just like x86).
> >>>>>>>>> So I think, at least for default config(which will be used by
> >>>>>>>>> distribution) to support max
> >>>>>>>>> lcores as fixed. I am not sure this patch changes those
> >>>>>>>>> aspects or not? Please check.
> >>>>>>>>
> >>>>>>>> This patch does *not* affect ‘default’ build type and cross-
> compilation.
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >>>>>>>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> >>>>>>>>>> ---
> >>>>>>>>>> config/get_max_lcores.py | 13 +++++++++++++
> >>>>>>>>>> config/meson.build       | 13 ++++++++++++-
> >>>>>>>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode
> >>>>>>>>>> 100755 config/get_max_lcores.py
> >>>>>>>>>>
> >>>>>>>>>> diff --git a/config/get_max_lcores.py
> >>>>>>>>>> b/config/get_max_lcores.py new file mode 100755 index
> >>>>>>>>>> 000000000000..ebf1c7efdadd
> >>>>>>>>>> --- /dev/null
> >>>>>>>>>> +++ b/config/get_max_lcores.py
> >>>>>>>>>> @@ -0,0 +1,13 @@
> >>>>>>>>>> +#!/usr/bin/python3
> >>>>>>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020
> >>>>>>>>>> +Arm Limited
> >>>>>>>>>> +
> >>>>>>>>>> +import os
> >>>>>>>>>> +
> >>>>>>>>>> +max_lcores = []
> >>>>>>>>>> +
> >>>>>>>>>> +nCPU = os.cpu_count()
> >>>>>>>>>> +
> >>>>>>>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
> >>>>>>>>>> +
> >>>>>>>>>> +print(' '.join(max_lcores))
> >>>>>>>>>> diff --git a/config/meson.build b/config/meson.build index
> >>>>>>>>>> 6996e5cbeaa5..80c05bc15d2f 100644
> >>>>>>>>>> --- a/config/meson.build
> >>>>>>>>>> +++ b/config/meson.build
> >>>>>>>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller
> >>>>>>>>>> reserved memory
> >>>>>>>> areas
> >>>>>>>>>>     dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
> >>>>>>>>>>
> >>>>>>>>>> -
> >>>>>>>>>> compile_time_cpuflags = []
> >>>>>>>>>> subdir(arch_subdir)
> >>>>>>>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
> >>>>>>>>>> ','.join(compile_time_cpuflags))
> >>>>>>>>>>
> >>>>>>>>>> +# set max lcores
> >>>>>>>>>> +if machine != 'default' and not meson.is_cross_build()
> >>>>>>>>>> +       # The script returns max lcores
> >>>>>>>>>> +       params = files('get_max_lcores.py')
> >>>>>>>>>> +       cmd_out = run_command(params)
> >>>>>>>
> >>>>>>> Have you considered running just a shell command, such as "nproc --
> all"?
> >>>>>>
> >>>>>> Is this really a good idea?
> >>>>>> For real distributions and NFV products, the build and runtime
> >>>>>> environment will usually be different even if on same CPU architecture.
> >>>>>>
> >>>>>> In many cases there maybe a huge build machine (128 CPU) or in a
> >>>>>> container (reported as single cpu) even if not doing cross build.
> >>>>>
> >>>>> That’s a great point, Stephen. IMO, this patch is useful when
> >>>>> building and running natively.
> >>>>> For all other purposes (like the ones you mentioned), do you think
> >>>>> it is a good idea to set RTE_MAX_LCORE using -Dmax_lcores?
> >>>>
> >>>> We should only use this native builds, as that would be consistent
> >>>> with the
> >> current meson build philosophy of "meson figuring as much as possible
> >> on its own". Any build other than native implies the user wants to
> >> deviate from the build machine.
> >>>>
> >>>
> >>> The MIDR value-based probing doesn’t quite work well for Arm IP
> >>> (currently
> >> being discussed on this patch: https://patches.dpdk.org/patch/76981/).
> >>>
> >>>> One way to do this automatic core count is when max_lcores=0 (0
> >>>> would
> >> have the special meaning of "figure core count automatically"). We
> >> can set that as default in meson_option.txt and then users will have
> >> the ability to set it to whatever they want, even for native builds. What do
> you think?
> >>>>
> >>>
> >>> Yes, agreed.
> >>>
> >>>> Currently the -Dmax_lcores option doesn't work for arm builds (the
> >>>> value of
> >> RTE_MAX_LCORE is overwritten in config/arm/meson.build). I believe
> >> the patch tries to address this, but still, we need to be mindful of that.
> >>>>
> >>>> Juraj
> >>
> >>
> >

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Dharmik Thakkar]

> On Oct 14, 2020, at 1:53 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> 
> Hi Dharmik,
> 
>> -----Original Message-----
>> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
>> Sent: Tuesday, October 13, 2020 5:15 PM
>> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
>> Cc: thomas@monjalon.net; dev@dpdk.org; Stephen Hemminger
>> <stephen@networkplumber.org>; Jerin Jacob <jerinjacobk@gmail.com>; nd
>> <nd@arm.com>
>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
>> 
>> Hi Juraj,
>> 
>>> On Oct 13, 2020, at 9:58 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>> 
>>> I believe we're going to drop this patch series in favor of
>> http://patches.dpdk.org/project/dpdk/list/?series=12923.
>> 
>> I can see you have included this feature in your series. Thank you!
>> What are your thoughts on the other patch [1]? Do you plan on including that as
>> well in your series?
>> 
>> [1] 	[1/2] config/arm: avoid variable reuse
>> https://patches.dpdk.org/patch/75946/
>> 
> 
> I believe the general idea of your patch is alredy part of my patch series in this patch: http://patches.dpdk.org/patch/80572/

Great, thank you! I will drop these patches.

> 
>>> 
>>>> -----Original Message-----
>>>> From: Thomas Monjalon <thomas@monjalon.net>
>>>> Sent: Tuesday, October 13, 2020 4:32 PM
>>>> To: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
>>>> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; dev@dpdk.org; Stephen
>>>> Hemminger <stephen@networkplumber.org>; Jerin Jacob
>>>> <jerinjacobk@gmail.com>; nd <nd@arm.com>
>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
>>>> programmatically
>>>> 
>>>> Please, what is the conclusion here?
>>>> 
>>>> 
>>>> 18/09/2020 07:47, Dharmik Thakkar:
>>>>> 
>>>>>> On Sep 17, 2020, at 4:56 AM, Juraj Linkeš
>>>>>> <juraj.linkes@pantheon.tech>
>>>> wrote:
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> -----Original Message-----
>>>>>>> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
>>>>>>> Sent: Friday, September 4, 2020 7:44 AM
>>>>>>> To: Stephen Hemminger <stephen@networkplumber.org>
>>>>>>> Cc: Juraj Linkeš <juraj.linkes@pantheon.tech>; Jerin Jacob
>>>>>>> <jerinjacobk@gmail.com>; thomas@monjalon.net; dpdk-dev
>>>>>>> <dev@dpdk.org>; nd <nd@arm.com>
>>>>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
>>>>>>> programmatically
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>>> On Sep 3, 2020, at 5:52 PM, Stephen Hemminger
>>>>>>> <stephen@networkplumber.org> wrote:
>>>>>>>> 
>>>>>>>> On Thu, 3 Sep 2020 06:20:17 +0000 Juraj Linkeš
>>>>>>>> <juraj.linkes@pantheon.tech> wrote:
>>>>>>>> 
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>>>>>>>>>> Sent: Wednesday, August 26, 2020 6:56 AM
>>>>>>>>>> To: Jerin Jacob <jerinjacobk@gmail.com>
>>>>>>>>>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd
>>>>>>>>>> <nd@arm.com>
>>>>>>>>>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore
>>>>>>>>>> programmatically
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob
>>>>>>>>>>> <jerinjacobk@gmail.com>
>>>>>>> wrote:
>>>>>>>>>>> 
>>>>>>>>>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
>>>>>>>>>> <dharmik.thakkar@arm.com> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It
>>>>>>>>>>>> leads to incorrect RTE_MAX_LCORE when machines have same
>>>> Implemener
>>>>>>>>>>>> and part number but different number of CPUs.
>>>>>>>>>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value
>>>>>>>>>>>> set in
>>>>>>>>>>>> meson_options.txt)
>>>>>>>>>>>> 
>>>>>>>>>>>> Use python script to find max lcore when using native build
>>>>>>>>>>>> to correctly set RTE_MAX_LCORE.
>>>>>>>>>>> 
>>>>>>>>>>> We may need to build on the native arm64 machine and use it on
>>>>>>>>>>> another
>>>>>>>>>>> arm64 machine(Just like x86).
>>>>>>>>>>> So I think, at least for default config(which will be used by
>>>>>>>>>>> distribution) to support max
>>>>>>>>>>> lcores as fixed. I am not sure this patch changes those
>>>>>>>>>>> aspects or not? Please check.
>>>>>>>>>> 
>>>>>>>>>> This patch does *not* affect ‘default’ build type and cross-
>> compilation.
>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>>>>>>>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>>>>>>>>>>>> ---
>>>>>>>>>>>> config/get_max_lcores.py | 13 +++++++++++++
>>>>>>>>>>>> config/meson.build       | 13 ++++++++++++-
>>>>>>>>>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode
>>>>>>>>>>>> 100755 config/get_max_lcores.py
>>>>>>>>>>>> 
>>>>>>>>>>>> diff --git a/config/get_max_lcores.py
>>>>>>>>>>>> b/config/get_max_lcores.py new file mode 100755 index
>>>>>>>>>>>> 000000000000..ebf1c7efdadd
>>>>>>>>>>>> --- /dev/null
>>>>>>>>>>>> +++ b/config/get_max_lcores.py
>>>>>>>>>>>> @@ -0,0 +1,13 @@
>>>>>>>>>>>> +#!/usr/bin/python3
>>>>>>>>>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020
>>>>>>>>>>>> +Arm Limited
>>>>>>>>>>>> +
>>>>>>>>>>>> +import os
>>>>>>>>>>>> +
>>>>>>>>>>>> +max_lcores = []
>>>>>>>>>>>> +
>>>>>>>>>>>> +nCPU = os.cpu_count()
>>>>>>>>>>>> +
>>>>>>>>>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
>>>>>>>>>>>> +
>>>>>>>>>>>> +print(' '.join(max_lcores))
>>>>>>>>>>>> diff --git a/config/meson.build b/config/meson.build index
>>>>>>>>>>>> 6996e5cbeaa5..80c05bc15d2f 100644
>>>>>>>>>>>> --- a/config/meson.build
>>>>>>>>>>>> +++ b/config/meson.build
>>>>>>>>>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller
>>>>>>>>>>>> reserved memory
>>>>>>>>>> areas
>>>>>>>>>>>>    dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
>>>>>>>>>>>> 
>>>>>>>>>>>> -
>>>>>>>>>>>> compile_time_cpuflags = []
>>>>>>>>>>>> subdir(arch_subdir)
>>>>>>>>>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
>>>>>>>>>>>> ','.join(compile_time_cpuflags))
>>>>>>>>>>>> 
>>>>>>>>>>>> +# set max lcores
>>>>>>>>>>>> +if machine != 'default' and not meson.is_cross_build()
>>>>>>>>>>>> +       # The script returns max lcores
>>>>>>>>>>>> +       params = files('get_max_lcores.py')
>>>>>>>>>>>> +       cmd_out = run_command(params)
>>>>>>>>> 
>>>>>>>>> Have you considered running just a shell command, such as "nproc --
>> all"?
>>>>>>>> 
>>>>>>>> Is this really a good idea?
>>>>>>>> For real distributions and NFV products, the build and runtime
>>>>>>>> environment will usually be different even if on same CPU architecture.
>>>>>>>> 
>>>>>>>> In many cases there maybe a huge build machine (128 CPU) or in a
>>>>>>>> container (reported as single cpu) even if not doing cross build.
>>>>>>> 
>>>>>>> That’s a great point, Stephen. IMO, this patch is useful when
>>>>>>> building and running natively.
>>>>>>> For all other purposes (like the ones you mentioned), do you think
>>>>>>> it is a good idea to set RTE_MAX_LCORE using -Dmax_lcores?
>>>>>> 
>>>>>> We should only use this native builds, as that would be consistent
>>>>>> with the
>>>> current meson build philosophy of "meson figuring as much as possible
>>>> on its own". Any build other than native implies the user wants to
>>>> deviate from the build machine.
>>>>>> 
>>>>> 
>>>>> The MIDR value-based probing doesn’t quite work well for Arm IP
>>>>> (currently
>>>> being discussed on this patch: https://patches.dpdk.org/patch/76981/).
>>>>> 
>>>>>> One way to do this automatic core count is when max_lcores=0 (0
>>>>>> would
>>>> have the special meaning of "figure core count automatically"). We
>>>> can set that as default in meson_option.txt and then users will have
>>>> the ability to set it to whatever they want, even for native builds. What do
>> you think?
>>>>>> 
>>>>> 
>>>>> Yes, agreed.
>>>>> 
>>>>>> Currently the -Dmax_lcores option doesn't work for arm builds (the
>>>>>> value of
>>>> RTE_MAX_LCORE is overwritten in config/arm/meson.build). I believe
>>>> the patch tries to address this, but still, we need to be mindful of that.
>>>>>> 
>>>>>> Juraj

Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically

[Dharmik Thakkar]

> On Sep 3, 2020, at 1:20 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>> Sent: Wednesday, August 26, 2020 6:56 AM
>> To: Jerin Jacob <jerinjacobk@gmail.com>
>> Cc: thomas@monjalon.net; dpdk-dev <dev@dpdk.org>; nd <nd@arm.com>
>> Subject: Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
>> 
>> 
>> 
>>> On Aug 25, 2020, at 11:47 PM, Jerin Jacob <jerinjacobk@gmail.com> wrote:
>>> 
>>> On Wed, Aug 26, 2020 at 2:44 AM Dharmik Thakkar
>> <dharmik.thakkar@arm.com> wrote:
>>>> 
>>>> For Arm, RTE_MAX_LCORE is hard-coded into the config. It leads to
>>>> incorrect RTE_MAX_LCORE when machines have same Implemener and part
>>>> number but different number of CPUs.
>>>> For x86, RTE_MAX_LCORE is always set to 128 (using the value set in
>>>> meson_options.txt)
>>>> 
>>>> Use python script to find max lcore when using native build to
>>>> correctly set RTE_MAX_LCORE.
>>> 
>>> We may need to build on the native arm64 machine and use it on another
>>> arm64 machine(Just like x86).
>>> So I think, at least for default config(which will be used by
>>> distribution) to support max
>>> lcores as fixed. I am not sure this patch changes those aspects or
>>> not? Please check.
>> 
>> This patch does *not* affect ‘default’ build type and cross-compilation.
>> 
>>> 
>>>> 
>>>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>>>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>>>> ---
>>>> config/get_max_lcores.py | 13 +++++++++++++
>>>> config/meson.build       | 13 ++++++++++++-
>>>> 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100755
>>>> config/get_max_lcores.py
>>>> 
>>>> diff --git a/config/get_max_lcores.py b/config/get_max_lcores.py new
>>>> file mode 100755 index 000000000000..ebf1c7efdadd
>>>> --- /dev/null
>>>> +++ b/config/get_max_lcores.py
>>>> @@ -0,0 +1,13 @@
>>>> +#!/usr/bin/python3
>>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Arm
>>>> +Limited
>>>> +
>>>> +import os
>>>> +
>>>> +max_lcores = []
>>>> +
>>>> +nCPU = os.cpu_count()
>>>> +
>>>> +max_lcores.append(str(nCPU & 0xFFF))             # Number of CPUs
>>>> +
>>>> +print(' '.join(max_lcores))
>>>> diff --git a/config/meson.build b/config/meson.build index
>>>> 6996e5cbeaa5..80c05bc15d2f 100644
>>>> --- a/config/meson.build
>>>> +++ b/config/meson.build
>>>> @@ -237,11 +237,22 @@ else # for 32-bit we need smaller reserved memory
>> areas
>>>>       dpdk_conf.set('RTE_MAX_MEM_MB', 2048) endif
>>>> 
>>>> -
>>>> compile_time_cpuflags = []
>>>> subdir(arch_subdir)
>>>> dpdk_conf.set('RTE_COMPILE_TIME_CPUFLAGS',
>>>> ','.join(compile_time_cpuflags))
>>>> 
>>>> +# set max lcores
>>>> +if machine != 'default' and not meson.is_cross_build()
>>>> +       # The script returns max lcores
>>>> +       params = files('get_max_lcores.py')
>>>> +       cmd_out = run_command(params)
> 
> Have you considered running just a shell command, such as "nproc --all"?

Shell command based solution such as “nproc —all” is not OS-agnostic, it doesn’t work on Windows.

> 
>>>> +       if cmd_out.returncode() == 0
>>>> +               cmd_lcore = cmd_out.stdout().to_lower().strip().split(' ')
>>>> +       endif
>>>> +       max_lcore = cmd_lcore[0].to_int()
>>>> +       dpdk_conf.set('RTE_MAX_LCORE', max_lcore) endif
>>>> +
>>>> # set the install path for the drivers
>>>> dpdk_conf.set_quoted('RTE_EAL_PMD_PATH', eal_pmd_path)
>>>> 
>>>> --
>>>> 2.17.1

Re: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse

[Juraj Linkeš]

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> Sent: Tuesday, August 25, 2020 11:13 PM
> To: Thomas Monjalon <thomas@monjalon.net>
> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> <dharmik.thakkar@arm.com>
> Subject: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse
> 
> Rename 'machine' to 'machine_properties' in config/arm/meson.build since
> 'machine' is previously being used in config/meson.build
> 

Wasn't the same variable name used by design? That is update the variable according to what's in config/arm/meson.build so that the new updated variable would be used in config/meson.build after that?

> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> ---
>  config/arm/meson.build | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> 8728051d5e38..e89ecdc4ccd2 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -141,12 +141,12 @@ else
>  	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
>  	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> 
> -	machine = []
> +	machine_properties = []
>  	cmd_generic = ['generic', '', '', 'default', '']
>  	cmd_output = cmd_generic # Set generic by default
>  	machine_args = [] # Clear previous machine args
>  	if arm_force_default_march and not meson.is_cross_build()
> -		machine = impl_generic
> +		machine_properties = impl_generic
>  		impl_pn = 'default'
>  	elif not meson.is_cross_build()
>  		# The script returns ['Implementer', 'Variant', 'Architecture', @@
> -158,9 +158,9 @@ else
>  			cmd_output = cmd.stdout().to_lower().strip().split(' ')
>  		endif
>  		# Set to generic if variable is not found
> -		machine = get_variable('impl_' + cmd_output[0], ['generic'])
> -		if machine[0] == 'generic'
> -			machine = impl_generic
> +		machine_properties = get_variable('impl_' + cmd_output[0],
> ['generic'])
> +		if machine_properties[0] == 'generic'
> +			machine_properties = impl_generic
>  			cmd_output = cmd_generic
>  		endif
>  		impl_pn = cmd_output[3]
> @@ -170,7 +170,7 @@ else
>  	else
>  		impl_id = meson.get_cross_property('implementor_id',
> 'generic')
>  		impl_pn = meson.get_cross_property('implementor_pn',
> 'default')
> -		machine = get_variable('impl_' + impl_id)
> +		machine_properties = get_variable('impl_' + impl_id)
>  	endif
> 
>  	# Apply Common Defaults. These settings may be overwritten by
> machine @@ -181,14 +181,14 @@ else
>  		endif
>  	endforeach
> 
> -	message('Implementer : ' + machine[0])
> -	foreach flag: machine[1]
> +	message('Implementer : ' + machine_properties[0])
> +	foreach flag: machine_properties[1]
>  		if flag.length() > 0
>  			dpdk_conf.set(flag[0], flag[1])
>  		endif
>  	endforeach
> 
> -	foreach marg: machine[2]
> +	foreach marg: machine_properties[2]
>  		if marg[0] == impl_pn
>  			foreach flag: marg[1]
>  				if cc.has_argument(flag)
> --
> 2.17.1
> 

Re: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse

[Dharmik Thakkar]

> On Sep 17, 2020, at 4:33 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
>> Sent: Tuesday, August 25, 2020 11:13 PM
>> To: Thomas Monjalon <thomas@monjalon.net>
>> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
>> <dharmik.thakkar@arm.com>
>> Subject: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse
>> 
>> Rename 'machine' to 'machine_properties' in config/arm/meson.build since
>> 'machine' is previously being used in config/meson.build
>> 
> 
> Wasn't the same variable name used by design? That is update the variable according to what's in config/arm/meson.build so that the new updated variable would be used in config/meson.build after that?
> 

The variable ‘machine’ within config/arm/meson.build is essentially machine + properties, so I think it makes sense to use a different variable name.
Also, since, ‘machine’ is a configuration option, IMO it is incorrect to update it.

>> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
>> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
>> ---
>> config/arm/meson.build | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>> 
>> diff --git a/config/arm/meson.build b/config/arm/meson.build index
>> 8728051d5e38..e89ecdc4ccd2 100644
>> --- a/config/arm/meson.build
>> +++ b/config/arm/meson.build
>> @@ -141,12 +141,12 @@ else
>> 	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
>> 	dpdk_conf.set('RTE_ARCH_ARM64', 1)
>> 
>> -	machine = []
>> +	machine_properties = []
>> 	cmd_generic = ['generic', '', '', 'default', '']
>> 	cmd_output = cmd_generic # Set generic by default
>> 	machine_args = [] # Clear previous machine args
>> 	if arm_force_default_march and not meson.is_cross_build()
>> -		machine = impl_generic
>> +		machine_properties = impl_generic
>> 		impl_pn = 'default'
>> 	elif not meson.is_cross_build()
>> 		# The script returns ['Implementer', 'Variant', 'Architecture', @@
>> -158,9 +158,9 @@ else
>> 			cmd_output = cmd.stdout().to_lower().strip().split(' ')
>> 		endif
>> 		# Set to generic if variable is not found
>> -		machine = get_variable('impl_' + cmd_output[0], ['generic'])
>> -		if machine[0] == 'generic'
>> -			machine = impl_generic
>> +		machine_properties = get_variable('impl_' + cmd_output[0],
>> ['generic'])
>> +		if machine_properties[0] == 'generic'
>> +			machine_properties = impl_generic
>> 			cmd_output = cmd_generic
>> 		endif
>> 		impl_pn = cmd_output[3]
>> @@ -170,7 +170,7 @@ else
>> 	else
>> 		impl_id = meson.get_cross_property('implementor_id',
>> 'generic')
>> 		impl_pn = meson.get_cross_property('implementor_pn',
>> 'default')
>> -		machine = get_variable('impl_' + impl_id)
>> +		machine_properties = get_variable('impl_' + impl_id)
>> 	endif
>> 
>> 	# Apply Common Defaults. These settings may be overwritten by
>> machine @@ -181,14 +181,14 @@ else
>> 		endif
>> 	endforeach
>> 
>> -	message('Implementer : ' + machine[0])
>> -	foreach flag: machine[1]
>> +	message('Implementer : ' + machine_properties[0])
>> +	foreach flag: machine_properties[1]
>> 		if flag.length() > 0
>> 			dpdk_conf.set(flag[0], flag[1])
>> 		endif
>> 	endforeach
>> 
>> -	foreach marg: machine[2]
>> +	foreach marg: machine_properties[2]
>> 		if marg[0] == impl_pn
>> 			foreach flag: marg[1]
>> 				if cc.has_argument(flag)
>> --
>> 2.17.1
>> 
> 
> 

Re: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse

[Juraj Linkeš]

> -----Original Message-----
> From: Dharmik Thakkar <Dharmik.Thakkar@arm.com>
> Sent: Friday, September 18, 2020 7:27 AM
> To: Juraj Linkeš <juraj.linkes@pantheon.tech>
> Cc: thomas@monjalon.net; dev@dpdk.org; nd <nd@arm.com>
> Subject: Re: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse
> 
> 
> 
> > On Sep 17, 2020, at 4:33 AM, Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
> >
> >
> >
> >> -----Original Message-----
> >> From: dev <dev-bounces@dpdk.org> On Behalf Of Dharmik Thakkar
> >> Sent: Tuesday, August 25, 2020 11:13 PM
> >> To: Thomas Monjalon <thomas@monjalon.net>
> >> Cc: dev@dpdk.org; nd@arm.com; Dharmik Thakkar
> >> <dharmik.thakkar@arm.com>
> >> Subject: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse
> >>
> >> Rename 'machine' to 'machine_properties' in config/arm/meson.build
> >> since 'machine' is previously being used in config/meson.build
> >>
> >
> > Wasn't the same variable name used by design? That is update the variable
> according to what's in config/arm/meson.build so that the new updated variable
> would be used in config/meson.build after that?
> >
> 
> The variable ‘machine’ within config/arm/meson.build is essentially machine +
> properties, so I think it makes sense to use a different variable name.
> Also, since, ‘machine’ is a configuration option, IMO it is incorrect to update it.
> 

It is a configuration option defined by us and only used in the config folder, so I guess the original author didn't think much of overwriting. It is actually used in the same manner in both config/meson.build and config/arm/meson.build and that is to set RTE_MACHINE, so I'd say it makes sense to use the same variable name from this point of view.

Also, I see that it's being overwritten in the code (although only for a special case):
if machine == 'default'
	if host_machine.cpu_family().startswith('x86')
		# matches the old pre-meson build systems default
		machine = 'corei7'

But it is only being overwritten in this special case and I think there is value in preserving the original configuration value. That leads me to the conclusion that using the same name was not a deliberate design choice (which was my original question). So, +1 from me.

> >> Signed-off-by: Dharmik Thakkar <dharmik.thakkar@arm.com>
> >> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> >> ---
> >> config/arm/meson.build | 18 +++++++++---------
> >> 1 file changed, 9 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> >> 8728051d5e38..e89ecdc4ccd2 100644
> >> --- a/config/arm/meson.build
> >> +++ b/config/arm/meson.build
> >> @@ -141,12 +141,12 @@ else
> >> 	dpdk_conf.set('RTE_CACHE_LINE_SIZE', 128)
> >> 	dpdk_conf.set('RTE_ARCH_ARM64', 1)
> >>
> >> -	machine = []
> >> +	machine_properties = []
> >> 	cmd_generic = ['generic', '', '', 'default', '']
> >> 	cmd_output = cmd_generic # Set generic by default
> >> 	machine_args = [] # Clear previous machine args
> >> 	if arm_force_default_march and not meson.is_cross_build()
> >> -		machine = impl_generic
> >> +		machine_properties = impl_generic
> >> 		impl_pn = 'default'
> >> 	elif not meson.is_cross_build()
> >> 		# The script returns ['Implementer', 'Variant', 'Architecture', @@
> >> -158,9 +158,9 @@ else
> >> 			cmd_output = cmd.stdout().to_lower().strip().split(' ')
> >> 		endif
> >> 		# Set to generic if variable is not found
> >> -		machine = get_variable('impl_' + cmd_output[0], ['generic'])
> >> -		if machine[0] == 'generic'
> >> -			machine = impl_generic
> >> +		machine_properties = get_variable('impl_' + cmd_output[0],
> >> ['generic'])
> >> +		if machine_properties[0] == 'generic'
> >> +			machine_properties = impl_generic
> >> 			cmd_output = cmd_generic
> >> 		endif
> >> 		impl_pn = cmd_output[3]
> >> @@ -170,7 +170,7 @@ else
> >> 	else
> >> 		impl_id = meson.get_cross_property('implementor_id',
> >> 'generic')
> >> 		impl_pn = meson.get_cross_property('implementor_pn',
> >> 'default')
> >> -		machine = get_variable('impl_' + impl_id)
> >> +		machine_properties = get_variable('impl_' + impl_id)
> >> 	endif
> >>
> >> 	# Apply Common Defaults. These settings may be overwritten by
> >> machine @@ -181,14 +181,14 @@ else
> >> 		endif
> >> 	endforeach
> >>
> >> -	message('Implementer : ' + machine[0])
> >> -	foreach flag: machine[1]
> >> +	message('Implementer : ' + machine_properties[0])
> >> +	foreach flag: machine_properties[1]
> >> 		if flag.length() > 0
> >> 			dpdk_conf.set(flag[0], flag[1])
> >> 		endif
> >> 	endforeach
> >>
> >> -	foreach marg: machine[2]
> >> +	foreach marg: machine_properties[2]
> >> 		if marg[0] == impl_pn
> >> 			foreach flag: marg[1]
> >> 				if cc.has_argument(flag)
> >> --
> >> 2.17.1
> >>
> >
> >

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>