DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse
@ 2020-08-25 21:13 Dharmik Thakkar
  2020-08-25 21:13 ` [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically Dharmik Thakkar
  2020-09-17  9:33 ` [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse Juraj Linkeš
  0 siblings, 2 replies; 18+ messages in thread
From: Dharmik Thakkar @ 2020-08-25 21:13 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, nd, 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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-08-25 21:13 [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse Dharmik Thakkar
@ 2020-08-25 21:13 ` Dharmik Thakkar
  2020-08-26  4:47   ` Jerin Jacob
  2020-09-17  9:33 ` [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse Juraj Linkeš
  1 sibling, 1 reply; 18+ messages in thread
From: Dharmik Thakkar @ 2020-08-25 21:13 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, nd, 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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-08-25 21:13 ` [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically Dharmik Thakkar
@ 2020-08-26  4:47   ` Jerin Jacob
  2020-08-26  4:55     ` Dharmik Thakkar
  0 siblings, 1 reply; 18+ messages in thread
From: Jerin Jacob @ 2020-08-26  4:47 UTC (permalink / raw)
  To: Dharmik Thakkar; +Cc: Thomas Monjalon, dpdk-dev, nd

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
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-08-26  4:47   ` Jerin Jacob
@ 2020-08-26  4:55     ` Dharmik Thakkar
  2020-09-03  6:20       ` Juraj Linkeš
  0 siblings, 1 reply; 18+ messages in thread
From: Dharmik Thakkar @ 2020-08-26  4:55 UTC (permalink / raw)
  To: Jerin Jacob; +Cc: thomas, dpdk-dev, nd



> 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
>> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-08-26  4:55     ` Dharmik Thakkar
@ 2020-09-03  6:20       ` Juraj Linkeš
  2020-09-03 22:52         ` Stephen Hemminger
  2020-09-04  5:26         ` Dharmik Thakkar
  0 siblings, 2 replies; 18+ messages in thread
From: Juraj Linkeš @ 2020-09-03  6:20 UTC (permalink / raw)
  To: Dharmik Thakkar, Jerin Jacob; +Cc: thomas, dpdk-dev, nd



> -----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
> >>


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-09-03  6:20       ` Juraj Linkeš
@ 2020-09-03 22:52         ` Stephen Hemminger
  2020-09-04  5:43           ` Dharmik Thakkar
  2020-09-04  5:26         ` Dharmik Thakkar
  1 sibling, 1 reply; 18+ messages in thread
From: Stephen Hemminger @ 2020-09-03 22:52 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: Dharmik Thakkar, Jerin Jacob, thomas, dpdk-dev, nd

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.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-09-03  6:20       ` Juraj Linkeš
  2020-09-03 22:52         ` Stephen Hemminger
@ 2020-09-04  5:26         ` Dharmik Thakkar
  1 sibling, 0 replies; 18+ messages in thread
From: Dharmik Thakkar @ 2020-09-04  5:26 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: Jerin Jacob, thomas, dpdk-dev, nd



> 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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-09-03 22:52         ` Stephen Hemminger
@ 2020-09-04  5:43           ` Dharmik Thakkar
  2020-09-17  9:56             ` Juraj Linkeš
  0 siblings, 1 reply; 18+ messages in thread
From: Dharmik Thakkar @ 2020-09-04  5:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Juraj Linkeš, Jerin Jacob, thomas, dpdk-dev, nd



> 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?


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse
  2020-08-25 21:13 [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse Dharmik Thakkar
  2020-08-25 21:13 ` [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically Dharmik Thakkar
@ 2020-09-17  9:33 ` Juraj Linkeš
  2020-09-18  5:26   ` Dharmik Thakkar
  1 sibling, 1 reply; 18+ messages in thread
From: Juraj Linkeš @ 2020-09-17  9:33 UTC (permalink / raw)
  To: Dharmik Thakkar, Thomas Monjalon; +Cc: dev, nd



> -----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
> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-09-04  5:43           ` Dharmik Thakkar
@ 2020-09-17  9:56             ` Juraj Linkeš
  2020-09-18  5:47               ` Dharmik Thakkar
  0 siblings, 1 reply; 18+ messages in thread
From: Juraj Linkeš @ 2020-09-17  9:56 UTC (permalink / raw)
  To: Dharmik Thakkar, Stephen Hemminger; +Cc: Jerin Jacob, thomas, dpdk-dev, nd



> -----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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse
  2020-09-17  9:33 ` [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse Juraj Linkeš
@ 2020-09-18  5:26   ` Dharmik Thakkar
  2020-09-18  8:40     ` Juraj Linkeš
  0 siblings, 1 reply; 18+ messages in thread
From: Dharmik Thakkar @ 2020-09-18  5:26 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: thomas, dev, nd



> 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
>> 
> 
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-09-17  9:56             ` Juraj Linkeš
@ 2020-09-18  5:47               ` Dharmik Thakkar
  2020-10-13 14:31                 ` Thomas Monjalon
  0 siblings, 1 reply; 18+ messages in thread
From: Dharmik Thakkar @ 2020-09-18  5:47 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: Stephen Hemminger, Jerin Jacob, thomas, dpdk-dev, nd



> 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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse
  2020-09-18  5:26   ` Dharmik Thakkar
@ 2020-09-18  8:40     ` Juraj Linkeš
  0 siblings, 0 replies; 18+ messages in thread
From: Juraj Linkeš @ 2020-09-18  8:40 UTC (permalink / raw)
  To: Dharmik Thakkar; +Cc: thomas, dev, nd



> -----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>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-09-18  5:47               ` Dharmik Thakkar
@ 2020-10-13 14:31                 ` Thomas Monjalon
  2020-10-13 14:58                   ` Juraj Linkeš
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Monjalon @ 2020-10-13 14:31 UTC (permalink / raw)
  To: Dharmik Thakkar
  Cc: Juraj Linkeš, dev, Stephen Hemminger, Jerin Jacob, nd

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



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-10-13 14:31                 ` Thomas Monjalon
@ 2020-10-13 14:58                   ` Juraj Linkeš
  2020-10-13 15:14                     ` Dharmik Thakkar
  0 siblings, 1 reply; 18+ messages in thread
From: Juraj Linkeš @ 2020-10-13 14:58 UTC (permalink / raw)
  To: Thomas Monjalon, Dharmik Thakkar; +Cc: dev, Stephen Hemminger, Jerin Jacob, nd

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
> 
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-10-13 14:58                   ` Juraj Linkeš
@ 2020-10-13 15:14                     ` Dharmik Thakkar
  2020-10-14  6:53                       ` Juraj Linkeš
  0 siblings, 1 reply; 18+ messages in thread
From: Dharmik Thakkar @ 2020-10-13 15:14 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: thomas, dev, Stephen Hemminger, Jerin Jacob, nd

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
>> 
>> 
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-10-13 15:14                     ` Dharmik Thakkar
@ 2020-10-14  6:53                       ` Juraj Linkeš
  2020-10-14 13:28                         ` Dharmik Thakkar
  0 siblings, 1 reply; 18+ messages in thread
From: Juraj Linkeš @ 2020-10-14  6:53 UTC (permalink / raw)
  To: Dharmik Thakkar; +Cc: thomas, dev, Stephen Hemminger, Jerin Jacob, nd

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
> >>
> >>
> >


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically
  2020-10-14  6:53                       ` Juraj Linkeš
@ 2020-10-14 13:28                         ` Dharmik Thakkar
  0 siblings, 0 replies; 18+ messages in thread
From: Dharmik Thakkar @ 2020-10-14 13:28 UTC (permalink / raw)
  To: Juraj Linkeš; +Cc: thomas, dev, Stephen Hemminger, Jerin Jacob, nd



> 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


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2020-10-14 13:29 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-25 21:13 [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse Dharmik Thakkar
2020-08-25 21:13 ` [dpdk-dev] [PATCH 2/2] build: find max lcore programmatically Dharmik Thakkar
2020-08-26  4:47   ` Jerin Jacob
2020-08-26  4:55     ` Dharmik Thakkar
2020-09-03  6:20       ` Juraj Linkeš
2020-09-03 22:52         ` Stephen Hemminger
2020-09-04  5:43           ` Dharmik Thakkar
2020-09-17  9:56             ` Juraj Linkeš
2020-09-18  5:47               ` Dharmik Thakkar
2020-10-13 14:31                 ` Thomas Monjalon
2020-10-13 14:58                   ` Juraj Linkeš
2020-10-13 15:14                     ` Dharmik Thakkar
2020-10-14  6:53                       ` Juraj Linkeš
2020-10-14 13:28                         ` Dharmik Thakkar
2020-09-04  5:26         ` Dharmik Thakkar
2020-09-17  9:33 ` [dpdk-dev] [PATCH 1/2] config/arm: avoid variable reuse Juraj Linkeš
2020-09-18  5:26   ` Dharmik Thakkar
2020-09-18  8:40     ` Juraj Linkeš

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).