DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson
@ 2019-12-02  6:14 Xiaolong Ye
  2019-12-02  8:10 ` Igor Ryzhov
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Xiaolong Ye @ 2019-12-02  6:14 UTC (permalink / raw)
  To: Ferruh Yigit, Bruce Richardson; +Cc: dev, Xiaolong Ye, stable, iryzhov

kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
system, for cross-compilation case, users would specify it as local
kernel src dir like

/<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/

Current meson build would fail to compile kernel module if user specify
kernel_dir as above, this patch fixes this issue.

Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
Cc: stable@dpdk.org
Cc: iryzhov@nfware.com

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---
 kernel/linux/igb_uio/meson.build | 2 +-
 kernel/linux/kni/meson.build     | 2 +-
 kernel/linux/meson.build         | 4 ++--
 meson_options.txt                | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/linux/igb_uio/meson.build b/kernel/linux/igb_uio/meson.build
index fac404f07..e66218dae 100644
--- a/kernel/linux/igb_uio/meson.build
+++ b/kernel/linux/igb_uio/meson.build
@@ -8,7 +8,7 @@ mkfile = custom_target('igb_uio_makefile',
 custom_target('igb_uio',
 	input: ['igb_uio.c', 'Kbuild'],
 	output: 'igb_uio.ko',
-	command: ['make', '-C', kernel_dir + '/build',
+	command: ['make', '-C', kernel_dir,
 		'M=' + meson.current_build_dir(),
 		'src=' + meson.current_source_dir(),
 		'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
index 955eec949..9fce0c16e 100644
--- a/kernel/linux/kni/meson.build
+++ b/kernel/linux/kni/meson.build
@@ -13,7 +13,7 @@ kni_sources = files(
 custom_target('rte_kni',
 	input: kni_sources,
 	output: 'rte_kni.ko',
-	command: ['make', '-j4', '-C', kernel_dir + '/build',
+	command: ['make', '-j4', '-C', kernel_dir,
 		'M=' + meson.current_build_dir(),
 		'src=' + meson.current_source_dir(),
 		'MODULE_CFLAGS=-include ' + meson.source_root() + '/config/rte_config.h' +
diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
index 1796cc686..a37c95752 100644
--- a/kernel/linux/meson.build
+++ b/kernel/linux/meson.build
@@ -13,11 +13,11 @@ kernel_dir = get_option('kernel_dir')
 if kernel_dir == ''
 	# use default path for native builds
 	kernel_version = run_command('uname', '-r').stdout().strip()
-	kernel_dir = '/lib/modules/' + kernel_version
+	kernel_dir = '/lib/modules/' + kernel_version + '/build'
 endif
 
 # test running make in kernel directory, using "make kernelversion"
-make_returncode = run_command('make', '-sC', kernel_dir + '/build',
+make_returncode = run_command('make', '-sC', kernel_dir,
 		'kernelversion').returncode()
 if make_returncode != 0
 	warning('Cannot compile kernel modules as requested - are kernel headers installed?')
diff --git a/meson_options.txt b/meson_options.txt
index bc369d06c..7eba3b720 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -17,7 +17,7 @@ option('ibverbs_link', type: 'combo', choices : ['shared', 'dlopen'], value: 'sh
 option('include_subdir_arch', type: 'string', value: '',
 	description: 'subdirectory where to install arch-dependent headers')
 option('kernel_dir', type: 'string', value: '',
-	description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
+	description: 'Path to the kernel for building kernel modules. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
 option('lib_musdk_dir', type: 'string', value: '',
 	description: 'path to the MUSDK library installation directory')
 option('machine', type: 'string', value: 'native',
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson
  2019-12-02  6:14 [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson Xiaolong Ye
@ 2019-12-02  8:10 ` Igor Ryzhov
  2019-12-02  8:39   ` Ye Xiaolong
  2019-12-03  5:29 ` [dpdk-dev] [PATCH v2] " Xiaolong Ye
  2019-12-03 15:59 ` [dpdk-dev] [PATCH v3] " Xiaolong Ye
  2 siblings, 1 reply; 20+ messages in thread
From: Igor Ryzhov @ 2019-12-02  8:10 UTC (permalink / raw)
  To: Xiaolong Ye; +Cc: Ferruh Yigit, Bruce Richardson, dev, dpdk stable

Hi Xiaolong,

Nack from me. It's just an incorrect revert of my fix.
Kernel modules will be installed in wrong directory, just check install_dir
parameter in kni/meson.build and igb_uio/meson.build.

Igor

On Mon, Dec 2, 2019 at 9:18 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:

> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
> system, for cross-compilation case, users would specify it as local
> kernel src dir like
>
> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>
> Current meson build would fail to compile kernel module if user specify
> kernel_dir as above, this patch fixes this issue.
>
> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> Cc: stable@dpdk.org
> Cc: iryzhov@nfware.com
>
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> ---
>  kernel/linux/igb_uio/meson.build | 2 +-
>  kernel/linux/kni/meson.build     | 2 +-
>  kernel/linux/meson.build         | 4 ++--
>  meson_options.txt                | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/linux/igb_uio/meson.build
> b/kernel/linux/igb_uio/meson.build
> index fac404f07..e66218dae 100644
> --- a/kernel/linux/igb_uio/meson.build
> +++ b/kernel/linux/igb_uio/meson.build
> @@ -8,7 +8,7 @@ mkfile = custom_target('igb_uio_makefile',
>  custom_target('igb_uio',
>         input: ['igb_uio.c', 'Kbuild'],
>         output: 'igb_uio.ko',
> -       command: ['make', '-C', kernel_dir + '/build',
> +       command: ['make', '-C', kernel_dir,
>                 'M=' + meson.current_build_dir(),
>                 'src=' + meson.current_source_dir(),
>                 'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
> diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
> index 955eec949..9fce0c16e 100644
> --- a/kernel/linux/kni/meson.build
> +++ b/kernel/linux/kni/meson.build
> @@ -13,7 +13,7 @@ kni_sources = files(
>  custom_target('rte_kni',
>         input: kni_sources,
>         output: 'rte_kni.ko',
> -       command: ['make', '-j4', '-C', kernel_dir + '/build',
> +       command: ['make', '-j4', '-C', kernel_dir,
>                 'M=' + meson.current_build_dir(),
>                 'src=' + meson.current_source_dir(),
>                 'MODULE_CFLAGS=-include ' + meson.source_root() +
> '/config/rte_config.h' +
> diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
> index 1796cc686..a37c95752 100644
> --- a/kernel/linux/meson.build
> +++ b/kernel/linux/meson.build
> @@ -13,11 +13,11 @@ kernel_dir = get_option('kernel_dir')
>  if kernel_dir == ''
>         # use default path for native builds
>         kernel_version = run_command('uname', '-r').stdout().strip()
> -       kernel_dir = '/lib/modules/' + kernel_version
> +       kernel_dir = '/lib/modules/' + kernel_version + '/build'
>  endif
>
>  # test running make in kernel directory, using "make kernelversion"
> -make_returncode = run_command('make', '-sC', kernel_dir + '/build',
> +make_returncode = run_command('make', '-sC', kernel_dir,
>                 'kernelversion').returncode()
>  if make_returncode != 0
>         warning('Cannot compile kernel modules as requested - are kernel
> headers installed?')
> diff --git a/meson_options.txt b/meson_options.txt
> index bc369d06c..7eba3b720 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -17,7 +17,7 @@ option('ibverbs_link', type: 'combo', choices :
> ['shared', 'dlopen'], value: 'sh
>  option('include_subdir_arch', type: 'string', value: '',
>         description: 'subdirectory where to install arch-dependent
> headers')
>  option('kernel_dir', type: 'string', value: '',
> -       description: 'Path to the kernel for building kernel modules.
> Headers must be in $kernel_dir/build. Modules will be installed in
> $DEST_DIR/$kernel_dir/extra/dpdk.')
> +       description: 'Path to the kernel for building kernel modules.
> Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
>  option('lib_musdk_dir', type: 'string', value: '',
>         description: 'path to the MUSDK library installation directory')
>  option('machine', type: 'string', value: 'native',
> --
> 2.17.1
>
>

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

* Re: [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson
  2019-12-02  8:10 ` Igor Ryzhov
@ 2019-12-02  8:39   ` Ye Xiaolong
  2019-12-02  9:16     ` Igor Ryzhov
  0 siblings, 1 reply; 20+ messages in thread
From: Ye Xiaolong @ 2019-12-02  8:39 UTC (permalink / raw)
  To: Igor Ryzhov; +Cc: Ferruh Yigit, Bruce Richardson, dev, dpdk stable

Hi, Igor

Thanks for the feedback.

On 12/02, Igor Ryzhov wrote:
>Hi Xiaolong,
>
>Nack from me. It's just an incorrect revert of my fix.
>Kernel modules will be installed in wrong directory, just check install_dir

Is there any convention that we must install kernel modules to which directory?
And what about for cross compilation case? 

Current issue is that if I specify kernel_dir as one local linux src dir in meson_options.txt,
meson build would fail to compile kernel modules while setting RTE_KERNELDIR='/<xxx>/build_dir/target-x86_64_glibc/linux-x86_64/linux-4.19.81/'
before make works fine. Is there any subtlety I have missed?

Thanks,
Xiaolong

>parameter in kni/meson.build and igb_uio/meson.build.
>
>Igor
>
>On Mon, Dec 2, 2019 at 9:18 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
>
>> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
>> system, for cross-compilation case, users would specify it as local
>> kernel src dir like
>>
>> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>>
>> Current meson build would fail to compile kernel module if user specify
>> kernel_dir as above, this patch fixes this issue.
>>
>> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
>> Cc: stable@dpdk.org
>> Cc: iryzhov@nfware.com
>>
>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> ---
>>  kernel/linux/igb_uio/meson.build | 2 +-
>>  kernel/linux/kni/meson.build     | 2 +-
>>  kernel/linux/meson.build         | 4 ++--
>>  meson_options.txt                | 2 +-
>>  4 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/kernel/linux/igb_uio/meson.build
>> b/kernel/linux/igb_uio/meson.build
>> index fac404f07..e66218dae 100644
>> --- a/kernel/linux/igb_uio/meson.build
>> +++ b/kernel/linux/igb_uio/meson.build
>> @@ -8,7 +8,7 @@ mkfile = custom_target('igb_uio_makefile',
>>  custom_target('igb_uio',
>>         input: ['igb_uio.c', 'Kbuild'],
>>         output: 'igb_uio.ko',
>> -       command: ['make', '-C', kernel_dir + '/build',
>> +       command: ['make', '-C', kernel_dir,
>>                 'M=' + meson.current_build_dir(),
>>                 'src=' + meson.current_source_dir(),
>>                 'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
>> diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
>> index 955eec949..9fce0c16e 100644
>> --- a/kernel/linux/kni/meson.build
>> +++ b/kernel/linux/kni/meson.build
>> @@ -13,7 +13,7 @@ kni_sources = files(
>>  custom_target('rte_kni',
>>         input: kni_sources,
>>         output: 'rte_kni.ko',
>> -       command: ['make', '-j4', '-C', kernel_dir + '/build',
>> +       command: ['make', '-j4', '-C', kernel_dir,
>>                 'M=' + meson.current_build_dir(),
>>                 'src=' + meson.current_source_dir(),
>>                 'MODULE_CFLAGS=-include ' + meson.source_root() +
>> '/config/rte_config.h' +
>> diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
>> index 1796cc686..a37c95752 100644
>> --- a/kernel/linux/meson.build
>> +++ b/kernel/linux/meson.build
>> @@ -13,11 +13,11 @@ kernel_dir = get_option('kernel_dir')
>>  if kernel_dir == ''
>>         # use default path for native builds
>>         kernel_version = run_command('uname', '-r').stdout().strip()
>> -       kernel_dir = '/lib/modules/' + kernel_version
>> +       kernel_dir = '/lib/modules/' + kernel_version + '/build'
>>  endif
>>
>>  # test running make in kernel directory, using "make kernelversion"
>> -make_returncode = run_command('make', '-sC', kernel_dir + '/build',
>> +make_returncode = run_command('make', '-sC', kernel_dir,
>>                 'kernelversion').returncode()
>>  if make_returncode != 0
>>         warning('Cannot compile kernel modules as requested - are kernel
>> headers installed?')
>> diff --git a/meson_options.txt b/meson_options.txt
>> index bc369d06c..7eba3b720 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -17,7 +17,7 @@ option('ibverbs_link', type: 'combo', choices :
>> ['shared', 'dlopen'], value: 'sh
>>  option('include_subdir_arch', type: 'string', value: '',
>>         description: 'subdirectory where to install arch-dependent
>> headers')
>>  option('kernel_dir', type: 'string', value: '',
>> -       description: 'Path to the kernel for building kernel modules.
>> Headers must be in $kernel_dir/build. Modules will be installed in
>> $DEST_DIR/$kernel_dir/extra/dpdk.')
>> +       description: 'Path to the kernel for building kernel modules.
>> Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
>>  option('lib_musdk_dir', type: 'string', value: '',
>>         description: 'path to the MUSDK library installation directory')
>>  option('machine', type: 'string', value: 'native',
>> --
>> 2.17.1
>>
>>

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

* Re: [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson
  2019-12-02  8:39   ` Ye Xiaolong
@ 2019-12-02  9:16     ` Igor Ryzhov
  2019-12-02 11:34       ` Ye Xiaolong
  0 siblings, 1 reply; 20+ messages in thread
From: Igor Ryzhov @ 2019-12-02  9:16 UTC (permalink / raw)
  To: Ye Xiaolong; +Cc: Ferruh Yigit, Bruce Richardson, dev, dpdk stable

We should at least install it into /lib/modules/kernel-version. For
convenience, dpdk modules are installed into
/lib/modules/kernel-version/extra/dpdk.
In the cross-compilation case, you can use DEST_DIR to set some prefix.

I don't really see the issue here. The description clearly says that
headers must be in $kernel_dir/build which is usually a symlink
to /usr/src/linux-headers-kernel-version.
Just set kernel_dir correctly and there won't be compilation failure.

Igor

On Mon, Dec 2, 2019 at 11:43 AM Ye Xiaolong <xiaolong.ye@intel.com> wrote:

> Hi, Igor
>
> Thanks for the feedback.
>
> On 12/02, Igor Ryzhov wrote:
> >Hi Xiaolong,
> >
> >Nack from me. It's just an incorrect revert of my fix.
> >Kernel modules will be installed in wrong directory, just check
> install_dir
>
> Is there any convention that we must install kernel modules to which
> directory?
> And what about for cross compilation case?
>
> Current issue is that if I specify kernel_dir as one local linux src dir
> in meson_options.txt,
> meson build would fail to compile kernel modules while setting
> RTE_KERNELDIR='/<xxx>/build_dir/target-x86_64_glibc/linux-x86_64/linux-4.19.81/'
> before make works fine. Is there any subtlety I have missed?
>
> Thanks,
> Xiaolong
>
> >parameter in kni/meson.build and igb_uio/meson.build.
> >
> >Igor
> >
> >On Mon, Dec 2, 2019 at 9:18 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
> >
> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
> >> system, for cross-compilation case, users would specify it as local
> >> kernel src dir like
> >>
> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> >>
> >> Current meson build would fail to compile kernel module if user specify
> >> kernel_dir as above, this patch fixes this issue.
> >>
> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> >> Cc: stable@dpdk.org
> >> Cc: iryzhov@nfware.com
> >>
> >> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >> ---
> >>  kernel/linux/igb_uio/meson.build | 2 +-
> >>  kernel/linux/kni/meson.build     | 2 +-
> >>  kernel/linux/meson.build         | 4 ++--
> >>  meson_options.txt                | 2 +-
> >>  4 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/kernel/linux/igb_uio/meson.build
> >> b/kernel/linux/igb_uio/meson.build
> >> index fac404f07..e66218dae 100644
> >> --- a/kernel/linux/igb_uio/meson.build
> >> +++ b/kernel/linux/igb_uio/meson.build
> >> @@ -8,7 +8,7 @@ mkfile = custom_target('igb_uio_makefile',
> >>  custom_target('igb_uio',
> >>         input: ['igb_uio.c', 'Kbuild'],
> >>         output: 'igb_uio.ko',
> >> -       command: ['make', '-C', kernel_dir + '/build',
> >> +       command: ['make', '-C', kernel_dir,
> >>                 'M=' + meson.current_build_dir(),
> >>                 'src=' + meson.current_source_dir(),
> >>                 'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
> >> diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
> >> index 955eec949..9fce0c16e 100644
> >> --- a/kernel/linux/kni/meson.build
> >> +++ b/kernel/linux/kni/meson.build
> >> @@ -13,7 +13,7 @@ kni_sources = files(
> >>  custom_target('rte_kni',
> >>         input: kni_sources,
> >>         output: 'rte_kni.ko',
> >> -       command: ['make', '-j4', '-C', kernel_dir + '/build',
> >> +       command: ['make', '-j4', '-C', kernel_dir,
> >>                 'M=' + meson.current_build_dir(),
> >>                 'src=' + meson.current_source_dir(),
> >>                 'MODULE_CFLAGS=-include ' + meson.source_root() +
> >> '/config/rte_config.h' +
> >> diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
> >> index 1796cc686..a37c95752 100644
> >> --- a/kernel/linux/meson.build
> >> +++ b/kernel/linux/meson.build
> >> @@ -13,11 +13,11 @@ kernel_dir = get_option('kernel_dir')
> >>  if kernel_dir == ''
> >>         # use default path for native builds
> >>         kernel_version = run_command('uname', '-r').stdout().strip()
> >> -       kernel_dir = '/lib/modules/' + kernel_version
> >> +       kernel_dir = '/lib/modules/' + kernel_version + '/build'
> >>  endif
> >>
> >>  # test running make in kernel directory, using "make kernelversion"
> >> -make_returncode = run_command('make', '-sC', kernel_dir + '/build',
> >> +make_returncode = run_command('make', '-sC', kernel_dir,
> >>                 'kernelversion').returncode()
> >>  if make_returncode != 0
> >>         warning('Cannot compile kernel modules as requested - are kernel
> >> headers installed?')
> >> diff --git a/meson_options.txt b/meson_options.txt
> >> index bc369d06c..7eba3b720 100644
> >> --- a/meson_options.txt
> >> +++ b/meson_options.txt
> >> @@ -17,7 +17,7 @@ option('ibverbs_link', type: 'combo', choices :
> >> ['shared', 'dlopen'], value: 'sh
> >>  option('include_subdir_arch', type: 'string', value: '',
> >>         description: 'subdirectory where to install arch-dependent
> >> headers')
> >>  option('kernel_dir', type: 'string', value: '',
> >> -       description: 'Path to the kernel for building kernel modules.
> >> Headers must be in $kernel_dir/build. Modules will be installed in
> >> $DEST_DIR/$kernel_dir/extra/dpdk.')
> >> +       description: 'Path to the kernel for building kernel modules.
> >> Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
> >>  option('lib_musdk_dir', type: 'string', value: '',
> >>         description: 'path to the MUSDK library installation directory')
> >>  option('machine', type: 'string', value: 'native',
> >> --
> >> 2.17.1
> >>
> >>
>

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

* Re: [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson
  2019-12-02  9:16     ` Igor Ryzhov
@ 2019-12-02 11:34       ` Ye Xiaolong
  2019-12-02 12:08         ` Bruce Richardson
  0 siblings, 1 reply; 20+ messages in thread
From: Ye Xiaolong @ 2019-12-02 11:34 UTC (permalink / raw)
  To: Igor Ryzhov; +Cc: Ferruh Yigit, Bruce Richardson, dev, dpdk stable

On 12/02, Igor Ryzhov wrote:
>We should at least install it into /lib/modules/kernel-version. For
>convenience, dpdk modules are installed into
>/lib/modules/kernel-version/extra/dpdk.
>In the cross-compilation case, you can use DEST_DIR to set some prefix.
>
>I don't really see the issue here. The description clearly says that
>headers must be in $kernel_dir/build which is usually a symlink
>to /usr/src/linux-headers-kernel-version.
>Just set kernel_dir correctly and there won't be compilation failure.

I think for cross-compilation case, user should be allowed to specify any kernel
src dir (it doesn't have to be /lib/modules/kernel-version) in his local system 
as kernel_dir that doesn't contain the build dir, in this case, current meson
build will skip kernel module compilation.

Thanks,
Xiaolong

>
>Igor
>
>On Mon, Dec 2, 2019 at 11:43 AM Ye Xiaolong <xiaolong.ye@intel.com> wrote:
>
>> Hi, Igor
>>
>> Thanks for the feedback.
>>
>> On 12/02, Igor Ryzhov wrote:
>> >Hi Xiaolong,
>> >
>> >Nack from me. It's just an incorrect revert of my fix.
>> >Kernel modules will be installed in wrong directory, just check
>> install_dir
>>
>> Is there any convention that we must install kernel modules to which
>> directory?
>> And what about for cross compilation case?
>>
>> Current issue is that if I specify kernel_dir as one local linux src dir
>> in meson_options.txt,
>> meson build would fail to compile kernel modules while setting
>> RTE_KERNELDIR='/<xxx>/build_dir/target-x86_64_glibc/linux-x86_64/linux-4.19.81/'
>> before make works fine. Is there any subtlety I have missed?
>>
>> Thanks,
>> Xiaolong
>>
>> >parameter in kni/meson.build and igb_uio/meson.build.
>> >
>> >Igor
>> >
>> >On Mon, Dec 2, 2019 at 9:18 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
>> >
>> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
>> >> system, for cross-compilation case, users would specify it as local
>> >> kernel src dir like
>> >>
>> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>> >>
>> >> Current meson build would fail to compile kernel module if user specify
>> >> kernel_dir as above, this patch fixes this issue.
>> >>
>> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
>> >> Cc: stable@dpdk.org
>> >> Cc: iryzhov@nfware.com
>> >>
>> >> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> >> ---
>> >>  kernel/linux/igb_uio/meson.build | 2 +-
>> >>  kernel/linux/kni/meson.build     | 2 +-
>> >>  kernel/linux/meson.build         | 4 ++--
>> >>  meson_options.txt                | 2 +-
>> >>  4 files changed, 5 insertions(+), 5 deletions(-)
>> >>
>> >> diff --git a/kernel/linux/igb_uio/meson.build
>> >> b/kernel/linux/igb_uio/meson.build
>> >> index fac404f07..e66218dae 100644
>> >> --- a/kernel/linux/igb_uio/meson.build
>> >> +++ b/kernel/linux/igb_uio/meson.build
>> >> @@ -8,7 +8,7 @@ mkfile = custom_target('igb_uio_makefile',
>> >>  custom_target('igb_uio',
>> >>         input: ['igb_uio.c', 'Kbuild'],
>> >>         output: 'igb_uio.ko',
>> >> -       command: ['make', '-C', kernel_dir + '/build',
>> >> +       command: ['make', '-C', kernel_dir,
>> >>                 'M=' + meson.current_build_dir(),
>> >>                 'src=' + meson.current_source_dir(),
>> >>                 'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
>> >> diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
>> >> index 955eec949..9fce0c16e 100644
>> >> --- a/kernel/linux/kni/meson.build
>> >> +++ b/kernel/linux/kni/meson.build
>> >> @@ -13,7 +13,7 @@ kni_sources = files(
>> >>  custom_target('rte_kni',
>> >>         input: kni_sources,
>> >>         output: 'rte_kni.ko',
>> >> -       command: ['make', '-j4', '-C', kernel_dir + '/build',
>> >> +       command: ['make', '-j4', '-C', kernel_dir,
>> >>                 'M=' + meson.current_build_dir(),
>> >>                 'src=' + meson.current_source_dir(),
>> >>                 'MODULE_CFLAGS=-include ' + meson.source_root() +
>> >> '/config/rte_config.h' +
>> >> diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
>> >> index 1796cc686..a37c95752 100644
>> >> --- a/kernel/linux/meson.build
>> >> +++ b/kernel/linux/meson.build
>> >> @@ -13,11 +13,11 @@ kernel_dir = get_option('kernel_dir')
>> >>  if kernel_dir == ''
>> >>         # use default path for native builds
>> >>         kernel_version = run_command('uname', '-r').stdout().strip()
>> >> -       kernel_dir = '/lib/modules/' + kernel_version
>> >> +       kernel_dir = '/lib/modules/' + kernel_version + '/build'
>> >>  endif
>> >>
>> >>  # test running make in kernel directory, using "make kernelversion"
>> >> -make_returncode = run_command('make', '-sC', kernel_dir + '/build',
>> >> +make_returncode = run_command('make', '-sC', kernel_dir,
>> >>                 'kernelversion').returncode()
>> >>  if make_returncode != 0
>> >>         warning('Cannot compile kernel modules as requested - are kernel
>> >> headers installed?')
>> >> diff --git a/meson_options.txt b/meson_options.txt
>> >> index bc369d06c..7eba3b720 100644
>> >> --- a/meson_options.txt
>> >> +++ b/meson_options.txt
>> >> @@ -17,7 +17,7 @@ option('ibverbs_link', type: 'combo', choices :
>> >> ['shared', 'dlopen'], value: 'sh
>> >>  option('include_subdir_arch', type: 'string', value: '',
>> >>         description: 'subdirectory where to install arch-dependent
>> >> headers')
>> >>  option('kernel_dir', type: 'string', value: '',
>> >> -       description: 'Path to the kernel for building kernel modules.
>> >> Headers must be in $kernel_dir/build. Modules will be installed in
>> >> $DEST_DIR/$kernel_dir/extra/dpdk.')
>> >> +       description: 'Path to the kernel for building kernel modules.
>> >> Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
>> >>  option('lib_musdk_dir', type: 'string', value: '',
>> >>         description: 'path to the MUSDK library installation directory')
>> >>  option('machine', type: 'string', value: 'native',
>> >> --
>> >> 2.17.1
>> >>
>> >>
>>

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

* Re: [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson
  2019-12-02 11:34       ` Ye Xiaolong
@ 2019-12-02 12:08         ` Bruce Richardson
  2019-12-02 15:44           ` Ye Xiaolong
  2019-12-03  5:33           ` Ye Xiaolong
  0 siblings, 2 replies; 20+ messages in thread
From: Bruce Richardson @ 2019-12-02 12:08 UTC (permalink / raw)
  To: Ye Xiaolong; +Cc: Igor Ryzhov, Ferruh Yigit, dev, dpdk stable

On Mon, Dec 02, 2019 at 07:34:54PM +0800, Ye Xiaolong wrote:
> On 12/02, Igor Ryzhov wrote:
> >We should at least install it into /lib/modules/kernel-version. For
> >convenience, dpdk modules are installed into
> >/lib/modules/kernel-version/extra/dpdk.
> >In the cross-compilation case, you can use DEST_DIR to set some prefix.
> >
> >I don't really see the issue here. The description clearly says that
> >headers must be in $kernel_dir/build which is usually a symlink
> >to /usr/src/linux-headers-kernel-version.
> >Just set kernel_dir correctly and there won't be compilation failure.
> 
> I think for cross-compilation case, user should be allowed to specify any kernel
> src dir (it doesn't have to be /lib/modules/kernel-version) in his local system 
> as kernel_dir that doesn't contain the build dir, in this case, current meson
> build will skip kernel module compilation.
> 

I don't think we can take this change as the default, since the previous
fix was put in for good reason.

However, perhaps we can attempt to support both, using the checks below for
"make kernelversion" in kernel/linux/meson.build. We can attempt using the
directory with /build (as now) and then if that fails attempt without it (or
vice versa).

/Bruce

> 
> >
> >Igor
> >
> >On Mon, Dec 2, 2019 at 11:43 AM Ye Xiaolong <xiaolong.ye@intel.com> wrote:
> >
> >> Hi, Igor
> >>
> >> Thanks for the feedback.
> >>
> >> On 12/02, Igor Ryzhov wrote:
> >> >Hi Xiaolong,
> >> >
> >> >Nack from me. It's just an incorrect revert of my fix.
> >> >Kernel modules will be installed in wrong directory, just check
> >> install_dir
> >>
> >> Is there any convention that we must install kernel modules to which
> >> directory?
> >> And what about for cross compilation case?
> >>
> >> Current issue is that if I specify kernel_dir as one local linux src dir
> >> in meson_options.txt,
> >> meson build would fail to compile kernel modules while setting
> >> RTE_KERNELDIR='/<xxx>/build_dir/target-x86_64_glibc/linux-x86_64/linux-4.19.81/'
> >> before make works fine. Is there any subtlety I have missed?
> >>
> >> Thanks,
> >> Xiaolong
> >>
> >> >parameter in kni/meson.build and igb_uio/meson.build.
> >> >
> >> >Igor
> >> >
> >> >On Mon, Dec 2, 2019 at 9:18 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
> >> >
> >> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
> >> >> system, for cross-compilation case, users would specify it as local
> >> >> kernel src dir like
> >> >>
> >> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> >> >>
> >> >> Current meson build would fail to compile kernel module if user specify
> >> >> kernel_dir as above, this patch fixes this issue.
> >> >>
> >> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> >> >> Cc: stable@dpdk.org
> >> >> Cc: iryzhov@nfware.com
> >> >>
> >> >> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >> >> ---
> >> >>  kernel/linux/igb_uio/meson.build | 2 +-
> >> >>  kernel/linux/kni/meson.build     | 2 +-
> >> >>  kernel/linux/meson.build         | 4 ++--
> >> >>  meson_options.txt                | 2 +-
> >> >>  4 files changed, 5 insertions(+), 5 deletions(-)
> >> >>
> >> >> diff --git a/kernel/linux/igb_uio/meson.build
> >> >> b/kernel/linux/igb_uio/meson.build
> >> >> index fac404f07..e66218dae 100644
> >> >> --- a/kernel/linux/igb_uio/meson.build
> >> >> +++ b/kernel/linux/igb_uio/meson.build
> >> >> @@ -8,7 +8,7 @@ mkfile = custom_target('igb_uio_makefile',
> >> >>  custom_target('igb_uio',
> >> >>         input: ['igb_uio.c', 'Kbuild'],
> >> >>         output: 'igb_uio.ko',
> >> >> -       command: ['make', '-C', kernel_dir + '/build',
> >> >> +       command: ['make', '-C', kernel_dir,
> >> >>                 'M=' + meson.current_build_dir(),
> >> >>                 'src=' + meson.current_source_dir(),
> >> >>                 'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
> >> >> diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
> >> >> index 955eec949..9fce0c16e 100644
> >> >> --- a/kernel/linux/kni/meson.build
> >> >> +++ b/kernel/linux/kni/meson.build
> >> >> @@ -13,7 +13,7 @@ kni_sources = files(
> >> >>  custom_target('rte_kni',
> >> >>         input: kni_sources,
> >> >>         output: 'rte_kni.ko',
> >> >> -       command: ['make', '-j4', '-C', kernel_dir + '/build',
> >> >> +       command: ['make', '-j4', '-C', kernel_dir,
> >> >>                 'M=' + meson.current_build_dir(),
> >> >>                 'src=' + meson.current_source_dir(),
> >> >>                 'MODULE_CFLAGS=-include ' + meson.source_root() +
> >> >> '/config/rte_config.h' +
> >> >> diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
> >> >> index 1796cc686..a37c95752 100644
> >> >> --- a/kernel/linux/meson.build
> >> >> +++ b/kernel/linux/meson.build
> >> >> @@ -13,11 +13,11 @@ kernel_dir = get_option('kernel_dir')
> >> >>  if kernel_dir == ''
> >> >>         # use default path for native builds
> >> >>         kernel_version = run_command('uname', '-r').stdout().strip()
> >> >> -       kernel_dir = '/lib/modules/' + kernel_version
> >> >> +       kernel_dir = '/lib/modules/' + kernel_version + '/build'
> >> >>  endif
> >> >>
> >> >>  # test running make in kernel directory, using "make kernelversion"
> >> >> -make_returncode = run_command('make', '-sC', kernel_dir + '/build',
> >> >> +make_returncode = run_command('make', '-sC', kernel_dir,
> >> >>                 'kernelversion').returncode()
> >> >>  if make_returncode != 0
> >> >>         warning('Cannot compile kernel modules as requested - are kernel
> >> >> headers installed?')
> >> >> diff --git a/meson_options.txt b/meson_options.txt
> >> >> index bc369d06c..7eba3b720 100644
> >> >> --- a/meson_options.txt
> >> >> +++ b/meson_options.txt
> >> >> @@ -17,7 +17,7 @@ option('ibverbs_link', type: 'combo', choices :
> >> >> ['shared', 'dlopen'], value: 'sh
> >> >>  option('include_subdir_arch', type: 'string', value: '',
> >> >>         description: 'subdirectory where to install arch-dependent
> >> >> headers')
> >> >>  option('kernel_dir', type: 'string', value: '',
> >> >> -       description: 'Path to the kernel for building kernel modules.
> >> >> Headers must be in $kernel_dir/build. Modules will be installed in
> >> >> $DEST_DIR/$kernel_dir/extra/dpdk.')
> >> >> +       description: 'Path to the kernel for building kernel modules.
> >> >> Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
> >> >>  option('lib_musdk_dir', type: 'string', value: '',
> >> >>         description: 'path to the MUSDK library installation directory')
> >> >>  option('machine', type: 'string', value: 'native',
> >> >> --
> >> >> 2.17.1
> >> >>
> >> >>
> >>

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

* Re: [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson
  2019-12-02 12:08         ` Bruce Richardson
@ 2019-12-02 15:44           ` Ye Xiaolong
  2019-12-03  5:33           ` Ye Xiaolong
  1 sibling, 0 replies; 20+ messages in thread
From: Ye Xiaolong @ 2019-12-02 15:44 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: Igor Ryzhov, Ferruh Yigit, dev, dpdk stable

On 12/02, Bruce Richardson wrote:
>On Mon, Dec 02, 2019 at 07:34:54PM +0800, Ye Xiaolong wrote:
>> On 12/02, Igor Ryzhov wrote:
>> >We should at least install it into /lib/modules/kernel-version. For
>> >convenience, dpdk modules are installed into
>> >/lib/modules/kernel-version/extra/dpdk.
>> >In the cross-compilation case, you can use DEST_DIR to set some prefix.
>> >
>> >I don't really see the issue here. The description clearly says that
>> >headers must be in $kernel_dir/build which is usually a symlink
>> >to /usr/src/linux-headers-kernel-version.
>> >Just set kernel_dir correctly and there won't be compilation failure.
>> 
>> I think for cross-compilation case, user should be allowed to specify any kernel
>> src dir (it doesn't have to be /lib/modules/kernel-version) in his local system 
>> as kernel_dir that doesn't contain the build dir, in this case, current meson
>> build will skip kernel module compilation.
>> 
>
>I don't think we can take this change as the default, since the previous
>fix was put in for good reason.
>
>However, perhaps we can attempt to support both, using the checks below for
>"make kernelversion" in kernel/linux/meson.build. We can attempt using the
>directory with /build (as now) and then if that fails attempt without it (or
>vice versa).

Thanks for the suggestion, I think it works for checking phase, and we still
some tricks for "make -C <kernel_dir>" to let it works for both cases, I'll try
to figure out.

Thanks,
Xiaolong

>
>/Bruce
>
>> 
>> >
>> >Igor
>> >
>> >On Mon, Dec 2, 2019 at 11:43 AM Ye Xiaolong <xiaolong.ye@intel.com> wrote:
>> >
>> >> Hi, Igor
>> >>
>> >> Thanks for the feedback.
>> >>
>> >> On 12/02, Igor Ryzhov wrote:
>> >> >Hi Xiaolong,
>> >> >
>> >> >Nack from me. It's just an incorrect revert of my fix.
>> >> >Kernel modules will be installed in wrong directory, just check
>> >> install_dir
>> >>
>> >> Is there any convention that we must install kernel modules to which
>> >> directory?
>> >> And what about for cross compilation case?
>> >>
>> >> Current issue is that if I specify kernel_dir as one local linux src dir
>> >> in meson_options.txt,
>> >> meson build would fail to compile kernel modules while setting
>> >> RTE_KERNELDIR='/<xxx>/build_dir/target-x86_64_glibc/linux-x86_64/linux-4.19.81/'
>> >> before make works fine. Is there any subtlety I have missed?
>> >>
>> >> Thanks,
>> >> Xiaolong
>> >>
>> >> >parameter in kni/meson.build and igb_uio/meson.build.
>> >> >
>> >> >Igor
>> >> >
>> >> >On Mon, Dec 2, 2019 at 9:18 AM Xiaolong Ye <xiaolong.ye@intel.com> wrote:
>> >> >
>> >> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
>> >> >> system, for cross-compilation case, users would specify it as local
>> >> >> kernel src dir like
>> >> >>
>> >> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>> >> >>
>> >> >> Current meson build would fail to compile kernel module if user specify
>> >> >> kernel_dir as above, this patch fixes this issue.
>> >> >>
>> >> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
>> >> >> Cc: stable@dpdk.org
>> >> >> Cc: iryzhov@nfware.com
>> >> >>
>> >> >> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> >> >> ---
>> >> >>  kernel/linux/igb_uio/meson.build | 2 +-
>> >> >>  kernel/linux/kni/meson.build     | 2 +-
>> >> >>  kernel/linux/meson.build         | 4 ++--
>> >> >>  meson_options.txt                | 2 +-
>> >> >>  4 files changed, 5 insertions(+), 5 deletions(-)
>> >> >>
>> >> >> diff --git a/kernel/linux/igb_uio/meson.build
>> >> >> b/kernel/linux/igb_uio/meson.build
>> >> >> index fac404f07..e66218dae 100644
>> >> >> --- a/kernel/linux/igb_uio/meson.build
>> >> >> +++ b/kernel/linux/igb_uio/meson.build
>> >> >> @@ -8,7 +8,7 @@ mkfile = custom_target('igb_uio_makefile',
>> >> >>  custom_target('igb_uio',
>> >> >>         input: ['igb_uio.c', 'Kbuild'],
>> >> >>         output: 'igb_uio.ko',
>> >> >> -       command: ['make', '-C', kernel_dir + '/build',
>> >> >> +       command: ['make', '-C', kernel_dir,
>> >> >>                 'M=' + meson.current_build_dir(),
>> >> >>                 'src=' + meson.current_source_dir(),
>> >> >>                 'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
>> >> >> diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
>> >> >> index 955eec949..9fce0c16e 100644
>> >> >> --- a/kernel/linux/kni/meson.build
>> >> >> +++ b/kernel/linux/kni/meson.build
>> >> >> @@ -13,7 +13,7 @@ kni_sources = files(
>> >> >>  custom_target('rte_kni',
>> >> >>         input: kni_sources,
>> >> >>         output: 'rte_kni.ko',
>> >> >> -       command: ['make', '-j4', '-C', kernel_dir + '/build',
>> >> >> +       command: ['make', '-j4', '-C', kernel_dir,
>> >> >>                 'M=' + meson.current_build_dir(),
>> >> >>                 'src=' + meson.current_source_dir(),
>> >> >>                 'MODULE_CFLAGS=-include ' + meson.source_root() +
>> >> >> '/config/rte_config.h' +
>> >> >> diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
>> >> >> index 1796cc686..a37c95752 100644
>> >> >> --- a/kernel/linux/meson.build
>> >> >> +++ b/kernel/linux/meson.build
>> >> >> @@ -13,11 +13,11 @@ kernel_dir = get_option('kernel_dir')
>> >> >>  if kernel_dir == ''
>> >> >>         # use default path for native builds
>> >> >>         kernel_version = run_command('uname', '-r').stdout().strip()
>> >> >> -       kernel_dir = '/lib/modules/' + kernel_version
>> >> >> +       kernel_dir = '/lib/modules/' + kernel_version + '/build'
>> >> >>  endif
>> >> >>
>> >> >>  # test running make in kernel directory, using "make kernelversion"
>> >> >> -make_returncode = run_command('make', '-sC', kernel_dir + '/build',
>> >> >> +make_returncode = run_command('make', '-sC', kernel_dir,
>> >> >>                 'kernelversion').returncode()
>> >> >>  if make_returncode != 0
>> >> >>         warning('Cannot compile kernel modules as requested - are kernel
>> >> >> headers installed?')
>> >> >> diff --git a/meson_options.txt b/meson_options.txt
>> >> >> index bc369d06c..7eba3b720 100644
>> >> >> --- a/meson_options.txt
>> >> >> +++ b/meson_options.txt
>> >> >> @@ -17,7 +17,7 @@ option('ibverbs_link', type: 'combo', choices :
>> >> >> ['shared', 'dlopen'], value: 'sh
>> >> >>  option('include_subdir_arch', type: 'string', value: '',
>> >> >>         description: 'subdirectory where to install arch-dependent
>> >> >> headers')
>> >> >>  option('kernel_dir', type: 'string', value: '',
>> >> >> -       description: 'Path to the kernel for building kernel modules.
>> >> >> Headers must be in $kernel_dir/build. Modules will be installed in
>> >> >> $DEST_DIR/$kernel_dir/extra/dpdk.')
>> >> >> +       description: 'Path to the kernel for building kernel modules.
>> >> >> Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
>> >> >>  option('lib_musdk_dir', type: 'string', value: '',
>> >> >>         description: 'path to the MUSDK library installation directory')
>> >> >>  option('machine', type: 'string', value: 'native',
>> >> >> --
>> >> >> 2.17.1
>> >> >>
>> >> >>
>> >>

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

* [dpdk-dev] [PATCH v2] kernel/linux: fix kernel dir for meson
  2019-12-02  6:14 [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson Xiaolong Ye
  2019-12-02  8:10 ` Igor Ryzhov
@ 2019-12-03  5:29 ` Xiaolong Ye
  2019-12-03 10:11   ` Bruce Richardson
  2019-12-03 15:59 ` [dpdk-dev] [PATCH v3] " Xiaolong Ye
  2 siblings, 1 reply; 20+ messages in thread
From: Xiaolong Ye @ 2019-12-03  5:29 UTC (permalink / raw)
  To: Ferruh Yigit, Bruce Richardson; +Cc: dev, Xiaolong Ye, stable, iryzhov

kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
system, for cross-compilation case, users would specify it as local
kernel src dir like

/<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/

Current meson build would fail to compile kernel module if user specify
kernel_dir as above, this patch fixes this issue.

Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
Cc: stable@dpdk.org
Cc: iryzhov@nfware.com

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---

V2 changes:

1. handle both normal and cross-compilation cases

 kernel/linux/igb_uio/meson.build |  4 ++--
 kernel/linux/kni/meson.build     |  4 ++--
 kernel/linux/meson.build         | 11 +++++++++--
 meson_options.txt                |  2 +-
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/kernel/linux/igb_uio/meson.build b/kernel/linux/igb_uio/meson.build
index fac404f07..63990372e 100644
--- a/kernel/linux/igb_uio/meson.build
+++ b/kernel/linux/igb_uio/meson.build
@@ -8,7 +8,7 @@ mkfile = custom_target('igb_uio_makefile',
 custom_target('igb_uio',
 	input: ['igb_uio.c', 'Kbuild'],
 	output: 'igb_uio.ko',
-	command: ['make', '-C', kernel_dir + '/build',
+	command: ['make', '-C', kernel_dir,
 		'M=' + meson.current_build_dir(),
 		'src=' + meson.current_source_dir(),
 		'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
@@ -16,5 +16,5 @@ custom_target('igb_uio',
 		'modules'],
 	depends: mkfile,
 	install: true,
-	install_dir: kernel_dir + '/extra/dpdk',
+	install_dir: install_base + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
index 955eec949..04c817e8a 100644
--- a/kernel/linux/kni/meson.build
+++ b/kernel/linux/kni/meson.build
@@ -13,7 +13,7 @@ kni_sources = files(
 custom_target('rte_kni',
 	input: kni_sources,
 	output: 'rte_kni.ko',
-	command: ['make', '-j4', '-C', kernel_dir + '/build',
+	command: ['make', '-j4', '-C', kernel_dir,
 		'M=' + meson.current_build_dir(),
 		'src=' + meson.current_source_dir(),
 		'MODULE_CFLAGS=-include ' + meson.source_root() + '/config/rte_config.h' +
@@ -25,5 +25,5 @@ custom_target('rte_kni',
 	depends: kni_mkfile,
 	console: true,
 	install: true,
-	install_dir: kernel_dir + '/extra/dpdk',
+	install_dir: install_base + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
index 1796cc686..db50f79fe 100644
--- a/kernel/linux/meson.build
+++ b/kernel/linux/meson.build
@@ -13,17 +13,24 @@ kernel_dir = get_option('kernel_dir')
 if kernel_dir == ''
 	# use default path for native builds
 	kernel_version = run_command('uname', '-r').stdout().strip()
-	kernel_dir = '/lib/modules/' + kernel_version
+	kernel_dir = '/lib/modules/' + kernel_version + '/build'
 endif
 
 # test running make in kernel directory, using "make kernelversion"
-make_returncode = run_command('make', '-sC', kernel_dir + '/build',
+make_returncode = run_command('make', '-sC', kernel_dir,
 		'kernelversion').returncode()
 if make_returncode != 0
 	warning('Cannot compile kernel modules as requested - are kernel headers installed?')
 	subdir_done()
 endif
 
+if kernel_dir.endswith('build')
+	install_base = kernel_dir.split('build')[0]
+else
+	install_base = kernel_dir
+endif
+
+
 # DO ACTUAL MODULE BUILDING
 foreach d:subdirs
 	subdir(d)
diff --git a/meson_options.txt b/meson_options.txt
index bc369d06c..7eba3b720 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -17,7 +17,7 @@ option('ibverbs_link', type: 'combo', choices : ['shared', 'dlopen'], value: 'sh
 option('include_subdir_arch', type: 'string', value: '',
 	description: 'subdirectory where to install arch-dependent headers')
 option('kernel_dir', type: 'string', value: '',
-	description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
+	description: 'Path to the kernel for building kernel modules. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
 option('lib_musdk_dir', type: 'string', value: '',
 	description: 'path to the MUSDK library installation directory')
 option('machine', type: 'string', value: 'native',
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson
  2019-12-02 12:08         ` Bruce Richardson
  2019-12-02 15:44           ` Ye Xiaolong
@ 2019-12-03  5:33           ` Ye Xiaolong
  2019-12-03 10:10             ` Bruce Richardson
  1 sibling, 1 reply; 20+ messages in thread
From: Ye Xiaolong @ 2019-12-03  5:33 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: Igor Ryzhov, Ferruh Yigit, dev, dpdk stable

On 12/02, Bruce Richardson wrote:
>On Mon, Dec 02, 2019 at 07:34:54PM +0800, Ye Xiaolong wrote:
>> On 12/02, Igor Ryzhov wrote:
>> >We should at least install it into /lib/modules/kernel-version. For
>> >convenience, dpdk modules are installed into
>> >/lib/modules/kernel-version/extra/dpdk.
>> >In the cross-compilation case, you can use DEST_DIR to set some prefix.
>> >
>> >I don't really see the issue here. The description clearly says that
>> >headers must be in $kernel_dir/build which is usually a symlink
>> >to /usr/src/linux-headers-kernel-version.
>> >Just set kernel_dir correctly and there won't be compilation failure.
>> 
>> I think for cross-compilation case, user should be allowed to specify any kernel
>> src dir (it doesn't have to be /lib/modules/kernel-version) in his local system 
>> as kernel_dir that doesn't contain the build dir, in this case, current meson
>> build will skip kernel module compilation.
>> 
>
>I don't think we can take this change as the default, since the previous
>fix was put in for good reason.
>
>However, perhaps we can attempt to support both, using the checks below for
>"make kernelversion" in kernel/linux/meson.build. We can attempt using the
>directory with /build (as now) and then if that fails attempt without it (or
>vice versa).

After a second thought, I think it'd be better that we unify the meaning of
kernel_dir for both cases, it should be aligned with make's RTE_KERNELDIR 
variable that specify the directory contains kernel src code (or header), then
we don't need to distinguish these 2 cases in check (make kernelversion) phase,
we just need to assign different install dirs,

For normal case:

kernel_dir=/lib/modules/<kernel_version>/build
install_dir=/lib/modules/<kernel_version>/extra/dpdk

For cross compilation case:

kernel_dir=<Any kernel src dir specified by user>
install_dir=<Any kernel src dir specified by user>/extra/dpdk

What do you think (I've sent v2 according to above description)?

Thanks,
Xiaolong

>
>/Bruce

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

* Re: [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson
  2019-12-03  5:33           ` Ye Xiaolong
@ 2019-12-03 10:10             ` Bruce Richardson
  0 siblings, 0 replies; 20+ messages in thread
From: Bruce Richardson @ 2019-12-03 10:10 UTC (permalink / raw)
  To: Ye Xiaolong; +Cc: Igor Ryzhov, Ferruh Yigit, dev, dpdk stable

On Tue, Dec 03, 2019 at 01:33:19PM +0800, Ye Xiaolong wrote:
> On 12/02, Bruce Richardson wrote:
> >On Mon, Dec 02, 2019 at 07:34:54PM +0800, Ye Xiaolong wrote:
> >> On 12/02, Igor Ryzhov wrote:
> >> >We should at least install it into /lib/modules/kernel-version. For
> >> >convenience, dpdk modules are installed into
> >> >/lib/modules/kernel-version/extra/dpdk.
> >> >In the cross-compilation case, you can use DEST_DIR to set some prefix.
> >> >
> >> >I don't really see the issue here. The description clearly says that
> >> >headers must be in $kernel_dir/build which is usually a symlink
> >> >to /usr/src/linux-headers-kernel-version.
> >> >Just set kernel_dir correctly and there won't be compilation failure.
> >> 
> >> I think for cross-compilation case, user should be allowed to specify any kernel
> >> src dir (it doesn't have to be /lib/modules/kernel-version) in his local system 
> >> as kernel_dir that doesn't contain the build dir, in this case, current meson
> >> build will skip kernel module compilation.
> >> 
> >
> >I don't think we can take this change as the default, since the previous
> >fix was put in for good reason.
> >
> >However, perhaps we can attempt to support both, using the checks below for
> >"make kernelversion" in kernel/linux/meson.build. We can attempt using the
> >directory with /build (as now) and then if that fails attempt without it (or
> >vice versa).
> 
> After a second thought, I think it'd be better that we unify the meaning of
> kernel_dir for both cases, it should be aligned with make's RTE_KERNELDIR 
> variable that specify the directory contains kernel src code (or header), then
> we don't need to distinguish these 2 cases in check (make kernelversion) phase,
> we just need to assign different install dirs,
> 
> For normal case:
> 
> kernel_dir=/lib/modules/<kernel_version>/build
> install_dir=/lib/modules/<kernel_version>/extra/dpdk
> 
> For cross compilation case:
> 
> kernel_dir=<Any kernel src dir specified by user>
> install_dir=<Any kernel src dir specified by user>/extra/dpdk
> 
> What do you think (I've sent v2 according to above description)?
> 

The downside of what you propose is that it will break any builds which are
already working by passing in the base kerneldir folder as parameter. That
case needs to be kept working, so you cannot force the user to pass in the
path with /build on the end.

/Bruce

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

* Re: [dpdk-dev] [PATCH v2] kernel/linux: fix kernel dir for meson
  2019-12-03  5:29 ` [dpdk-dev] [PATCH v2] " Xiaolong Ye
@ 2019-12-03 10:11   ` Bruce Richardson
  2019-12-03 12:33     ` Ye Xiaolong
  0 siblings, 1 reply; 20+ messages in thread
From: Bruce Richardson @ 2019-12-03 10:11 UTC (permalink / raw)
  To: Xiaolong Ye; +Cc: Ferruh Yigit, dev, stable, iryzhov

On Tue, Dec 03, 2019 at 01:29:17PM +0800, Xiaolong Ye wrote:
> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
> system, for cross-compilation case, users would specify it as local
> kernel src dir like
> 
> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> 
> Current meson build would fail to compile kernel module if user specify
> kernel_dir as above, this patch fixes this issue.
> 
> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> Cc: stable@dpdk.org
> Cc: iryzhov@nfware.com
> 
> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> ---
> 
> V2 changes:
> 
> 1. handle both normal and cross-compilation cases
> 
We need to handle both, but they need to be handled without breaking the
currently working case where we pass in /lib/modules/$(uname -r)/ as the
kerneldir path.

/Bruce

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

* Re: [dpdk-dev] [PATCH v2] kernel/linux: fix kernel dir for meson
  2019-12-03 10:11   ` Bruce Richardson
@ 2019-12-03 12:33     ` Ye Xiaolong
  2019-12-03 13:58       ` Bruce Richardson
  0 siblings, 1 reply; 20+ messages in thread
From: Ye Xiaolong @ 2019-12-03 12:33 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: Ferruh Yigit, dev, stable, iryzhov

On 12/03, Bruce Richardson wrote:
>On Tue, Dec 03, 2019 at 01:29:17PM +0800, Xiaolong Ye wrote:
>> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
>> system, for cross-compilation case, users would specify it as local
>> kernel src dir like
>> 
>> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>> 
>> Current meson build would fail to compile kernel module if user specify
>> kernel_dir as above, this patch fixes this issue.
>> 
>> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
>> Cc: stable@dpdk.org
>> Cc: iryzhov@nfware.com
>> 
>> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> ---
>> 
>> V2 changes:
>> 
>> 1. handle both normal and cross-compilation cases
>> 
>We need to handle both, but they need to be handled without breaking the
>currently working case where we pass in /lib/modules/$(uname -r)/ as the
>kerneldir path.

So you mean we should allow user to specify both /lib/modules/$(uname -r) and
/lib/modules/$(uname -r)/build as kernel_dir for normal case?

Thanks,
Xiaolong

>
>/Bruce

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

* Re: [dpdk-dev] [PATCH v2] kernel/linux: fix kernel dir for meson
  2019-12-03 12:33     ` Ye Xiaolong
@ 2019-12-03 13:58       ` Bruce Richardson
  2019-12-03 15:01         ` Ye Xiaolong
  0 siblings, 1 reply; 20+ messages in thread
From: Bruce Richardson @ 2019-12-03 13:58 UTC (permalink / raw)
  To: Ye Xiaolong; +Cc: Ferruh Yigit, dev, stable, iryzhov

On Tue, Dec 03, 2019 at 08:33:22PM +0800, Ye Xiaolong wrote:
> On 12/03, Bruce Richardson wrote:
> >On Tue, Dec 03, 2019 at 01:29:17PM +0800, Xiaolong Ye wrote:
> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
> >> system, for cross-compilation case, users would specify it as local
> >> kernel src dir like
> >> 
> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> >> 
> >> Current meson build would fail to compile kernel module if user specify
> >> kernel_dir as above, this patch fixes this issue.
> >> 
> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> >> Cc: stable@dpdk.org
> >> Cc: iryzhov@nfware.com
> >> 
> >> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
> >> ---
> >> 
> >> V2 changes:
> >> 
> >> 1. handle both normal and cross-compilation cases
> >> 
> >We need to handle both, but they need to be handled without breaking the
> >currently working case where we pass in /lib/modules/$(uname -r)/ as the
> >kerneldir path.
> 
> So you mean we should allow user to specify both /lib/modules/$(uname -r) and
> /lib/modules/$(uname -r)/build as kernel_dir for normal case?
> 
That is up to you, but we need to still allow the former case so as to
avoid breaking backward compatibility for existing build setups. Therefore
I suggest supporting both is recommended.

/Bruce

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

* Re: [dpdk-dev] [PATCH v2] kernel/linux: fix kernel dir for meson
  2019-12-03 13:58       ` Bruce Richardson
@ 2019-12-03 15:01         ` Ye Xiaolong
  0 siblings, 0 replies; 20+ messages in thread
From: Ye Xiaolong @ 2019-12-03 15:01 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: Ferruh Yigit, dev, stable, iryzhov

On 12/03, Bruce Richardson wrote:
>On Tue, Dec 03, 2019 at 08:33:22PM +0800, Ye Xiaolong wrote:
>> On 12/03, Bruce Richardson wrote:
>> >On Tue, Dec 03, 2019 at 01:29:17PM +0800, Xiaolong Ye wrote:
>> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
>> >> system, for cross-compilation case, users would specify it as local
>> >> kernel src dir like
>> >> 
>> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>> >> 
>> >> Current meson build would fail to compile kernel module if user specify
>> >> kernel_dir as above, this patch fixes this issue.
>> >> 
>> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
>> >> Cc: stable@dpdk.org
>> >> Cc: iryzhov@nfware.com
>> >> 
>> >> Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
>> >> ---
>> >> 
>> >> V2 changes:
>> >> 
>> >> 1. handle both normal and cross-compilation cases
>> >> 
>> >We need to handle both, but they need to be handled without breaking the
>> >currently working case where we pass in /lib/modules/$(uname -r)/ as the
>> >kerneldir path.
>> 
>> So you mean we should allow user to specify both /lib/modules/$(uname -r) and
>> /lib/modules/$(uname -r)/build as kernel_dir for normal case?
>> 
>That is up to you, but we need to still allow the former case so as to
>avoid breaking backward compatibility for existing build setups. Therefore
>I suggest supporting both is recommended.

Make sense, I'll try a new version.

Thanks,
Xiaolong
>
>/Bruce

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

* [dpdk-dev] [PATCH v3] kernel/linux: fix kernel dir for meson
  2019-12-02  6:14 [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson Xiaolong Ye
  2019-12-02  8:10 ` Igor Ryzhov
  2019-12-03  5:29 ` [dpdk-dev] [PATCH v2] " Xiaolong Ye
@ 2019-12-03 15:59 ` Xiaolong Ye
  2019-12-04 13:51   ` Luca Boccassi
  2 siblings, 1 reply; 20+ messages in thread
From: Xiaolong Ye @ 2019-12-03 15:59 UTC (permalink / raw)
  To: Ferruh Yigit, Bruce Richardson; +Cc: dev, Xiaolong Ye, stable, iryzhov

kernel_dir option in meson build is equivalent to RTE_KERNELDIR in make
system, for cross-compilation case, users would specify it as local
kernel src dir like

/<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/

Current meson build would fail to compile kernel module if user specify
kernel_dir as above, this patch fixes this issue.

After this change, for normal build case, user can specify
/lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build as
kernel_dir. For cross compilation case, user can specify any directory
that contains kernel source code as the kernel_dir.

Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
Cc: stable@dpdk.org
Cc: iryzhov@nfware.com

Signed-off-by: Xiaolong Ye <xiaolong.ye@intel.com>
---

V3 changes:

1. support setting both /lib/modules/<kernel_version> and
   /lib/modules/<kernel_version>/build as kernel_dir to keep backward
   compatibility

V2 changes:

1. handle both normal and cross-compilation cases

 kernel/linux/igb_uio/meson.build |  4 ++--
 kernel/linux/kni/meson.build     |  4 ++--
 kernel/linux/meson.build         | 19 +++++++++++++++----
 meson_options.txt                |  2 +-
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/kernel/linux/igb_uio/meson.build b/kernel/linux/igb_uio/meson.build
index fac404f07..63990372e 100644
--- a/kernel/linux/igb_uio/meson.build
+++ b/kernel/linux/igb_uio/meson.build
@@ -8,7 +8,7 @@ mkfile = custom_target('igb_uio_makefile',
 custom_target('igb_uio',
 	input: ['igb_uio.c', 'Kbuild'],
 	output: 'igb_uio.ko',
-	command: ['make', '-C', kernel_dir + '/build',
+	command: ['make', '-C', kernel_dir,
 		'M=' + meson.current_build_dir(),
 		'src=' + meson.current_source_dir(),
 		'EXTRA_CFLAGS=-I' + meson.current_source_dir() +
@@ -16,5 +16,5 @@ custom_target('igb_uio',
 		'modules'],
 	depends: mkfile,
 	install: true,
-	install_dir: kernel_dir + '/extra/dpdk',
+	install_dir: install_base + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
index 955eec949..04c817e8a 100644
--- a/kernel/linux/kni/meson.build
+++ b/kernel/linux/kni/meson.build
@@ -13,7 +13,7 @@ kni_sources = files(
 custom_target('rte_kni',
 	input: kni_sources,
 	output: 'rte_kni.ko',
-	command: ['make', '-j4', '-C', kernel_dir + '/build',
+	command: ['make', '-j4', '-C', kernel_dir,
 		'M=' + meson.current_build_dir(),
 		'src=' + meson.current_source_dir(),
 		'MODULE_CFLAGS=-include ' + meson.source_root() + '/config/rte_config.h' +
@@ -25,5 +25,5 @@ custom_target('rte_kni',
 	depends: kni_mkfile,
 	console: true,
 	install: true,
-	install_dir: kernel_dir + '/extra/dpdk',
+	install_dir: install_base + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
index 1796cc686..0568ed7e1 100644
--- a/kernel/linux/meson.build
+++ b/kernel/linux/meson.build
@@ -13,15 +13,26 @@ kernel_dir = get_option('kernel_dir')
 if kernel_dir == ''
 	# use default path for native builds
 	kernel_version = run_command('uname', '-r').stdout().strip()
-	kernel_dir = '/lib/modules/' + kernel_version
+	kernel_dir = '/lib/modules/' + kernel_version + '/build'
 endif
 
 # test running make in kernel directory, using "make kernelversion"
-make_returncode = run_command('make', '-sC', kernel_dir + '/build',
+make_returncode = run_command('make', '-sC', kernel_dir,
 		'kernelversion').returncode()
 if make_returncode != 0
-	warning('Cannot compile kernel modules as requested - are kernel headers installed?')
-	subdir_done()
+	kernel_dir = kernel_dir + '/build'
+	make_returncode = run_command('make', '-sC', kernel_dir,
+			'kernelversion').returncode()
+	if make_returncode != 0
+		warning('Cannot compile kernel modules as requested - are kernel headers installed?')
+		subdir_done()
+	endif
+endif
+
+if kernel_dir.endswith('build') or kernel_dir.endswith('build/')
+	install_base = kernel_dir.split('build')[0]
+else
+	install_base = kernel_dir
 endif
 
 # DO ACTUAL MODULE BUILDING
diff --git a/meson_options.txt b/meson_options.txt
index bc369d06c..7eba3b720 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -17,7 +17,7 @@ option('ibverbs_link', type: 'combo', choices : ['shared', 'dlopen'], value: 'sh
 option('include_subdir_arch', type: 'string', value: '',
 	description: 'subdirectory where to install arch-dependent headers')
 option('kernel_dir', type: 'string', value: '',
-	description: 'Path to the kernel for building kernel modules. Headers must be in $kernel_dir/build. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
+	description: 'Path to the kernel for building kernel modules. Modules will be installed in $DEST_DIR/$kernel_dir/extra/dpdk.')
 option('lib_musdk_dir', type: 'string', value: '',
 	description: 'path to the MUSDK library installation directory')
 option('machine', type: 'string', value: 'native',
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v3] kernel/linux: fix kernel dir for meson
  2019-12-03 15:59 ` [dpdk-dev] [PATCH v3] " Xiaolong Ye
@ 2019-12-04 13:51   ` Luca Boccassi
  2019-12-04 14:18     ` Ye Xiaolong
  0 siblings, 1 reply; 20+ messages in thread
From: Luca Boccassi @ 2019-12-04 13:51 UTC (permalink / raw)
  To: Xiaolong Ye, Ferruh Yigit, Bruce Richardson; +Cc: dev, stable, iryzhov

On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
> make
> system, for cross-compilation case, users would specify it as local
> kernel src dir like
> 
> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> 
> Current meson build would fail to compile kernel module if user
> specify
> kernel_dir as above, this patch fixes this issue.
> 
> After this change, for normal build case, user can specify
> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
> as
> kernel_dir. For cross compilation case, user can specify any
> directory
> that contains kernel source code as the kernel_dir.
> 
> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> Cc: 
> stable@dpdk.org
> 
> Cc: 
> iryzhov@nfware.com
> 
> 
> Signed-off-by: Xiaolong Ye <
> xiaolong.ye@intel.com

The convention used by upstream and all distros is that kernel headers
are in <version>/build. Why can't the cross compilation case also
follow this convention, rather than adding complications to the
downstream build system?

-- 
Kind regards,
Luca Boccassi

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

* Re: [dpdk-dev] [PATCH v3] kernel/linux: fix kernel dir for meson
  2019-12-04 13:51   ` Luca Boccassi
@ 2019-12-04 14:18     ` Ye Xiaolong
  2019-12-04 15:12       ` Bruce Richardson
  0 siblings, 1 reply; 20+ messages in thread
From: Ye Xiaolong @ 2019-12-04 14:18 UTC (permalink / raw)
  To: Luca Boccassi; +Cc: Ferruh Yigit, Bruce Richardson, dev, stable, iryzhov

On 12/04, Luca Boccassi wrote:
>On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
>> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
>> make
>> system, for cross-compilation case, users would specify it as local
>> kernel src dir like
>> 
>> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>> 
>> Current meson build would fail to compile kernel module if user
>> specify
>> kernel_dir as above, this patch fixes this issue.
>> 
>> After this change, for normal build case, user can specify
>> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
>> as
>> kernel_dir. For cross compilation case, user can specify any
>> directory
>> that contains kernel source code as the kernel_dir.
>> 
>> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
>> Cc: 
>> stable@dpdk.org
>> 
>> Cc: 
>> iryzhov@nfware.com
>> 
>> 
>> Signed-off-by: Xiaolong Ye <
>> xiaolong.ye@intel.com
>
>The convention used by upstream and all distros is that kernel headers
>are in <version>/build. Why can't the cross compilation case also
>follow this convention, rather than adding complications to the

Yes, cross-compilation can follow this convention, but one common case is that
users download and put kernel src (the same kernel that's running in the target machine)
to one arbitrary dir, he then use this dir as kernel_dir to build kernel modules,
it's extra burden for users to create extra build dir to hold the kernel headers.

Thanks,
Xiaolong

>downstream build system?
>
>-- 
>Kind regards,
>Luca Boccassi

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

* Re: [dpdk-dev] [PATCH v3] kernel/linux: fix kernel dir for meson
  2019-12-04 14:18     ` Ye Xiaolong
@ 2019-12-04 15:12       ` Bruce Richardson
  2019-12-08  1:26         ` Ye Xiaolong
  0 siblings, 1 reply; 20+ messages in thread
From: Bruce Richardson @ 2019-12-04 15:12 UTC (permalink / raw)
  To: Ye Xiaolong; +Cc: Luca Boccassi, Ferruh Yigit, dev, stable, iryzhov

On Wed, Dec 04, 2019 at 10:18:21PM +0800, Ye Xiaolong wrote:
> On 12/04, Luca Boccassi wrote:
> >On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
> >> make
> >> system, for cross-compilation case, users would specify it as local
> >> kernel src dir like
> >> 
> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> >> 
> >> Current meson build would fail to compile kernel module if user
> >> specify
> >> kernel_dir as above, this patch fixes this issue.
> >> 
> >> After this change, for normal build case, user can specify
> >> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
> >> as
> >> kernel_dir. For cross compilation case, user can specify any
> >> directory
> >> that contains kernel source code as the kernel_dir.
> >> 
> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> >> Cc: 
> >> stable@dpdk.org
> >> 
> >> Cc: 
> >> iryzhov@nfware.com
> >> 
> >> 
> >> Signed-off-by: Xiaolong Ye <
> >> xiaolong.ye@intel.com
> >
> >The convention used by upstream and all distros is that kernel headers
> >are in <version>/build. Why can't the cross compilation case also
> >follow this convention, rather than adding complications to the
> 
> Yes, cross-compilation can follow this convention, but one common case is that
> users download and put kernel src (the same kernel that's running in the target machine)
> to one arbitrary dir, he then use this dir as kernel_dir to build kernel modules,
> it's extra burden for users to create extra build dir to hold the kernel headers.
> 

As part of the build of the kernel, do you not do a "modules_install" step,
which should set up things correctly for later builds?

/Bruce

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

* Re: [dpdk-dev] [PATCH v3] kernel/linux: fix kernel dir for meson
  2019-12-04 15:12       ` Bruce Richardson
@ 2019-12-08  1:26         ` Ye Xiaolong
  2019-12-09 12:12           ` Bruce Richardson
  0 siblings, 1 reply; 20+ messages in thread
From: Ye Xiaolong @ 2019-12-08  1:26 UTC (permalink / raw)
  To: Bruce Richardson; +Cc: Luca Boccassi, Ferruh Yigit, dev, stable, iryzhov

On 12/04, Bruce Richardson wrote:
>On Wed, Dec 04, 2019 at 10:18:21PM +0800, Ye Xiaolong wrote:
>> On 12/04, Luca Boccassi wrote:
>> >On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
>> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
>> >> make
>> >> system, for cross-compilation case, users would specify it as local
>> >> kernel src dir like
>> >> 
>> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
>> >> 
>> >> Current meson build would fail to compile kernel module if user
>> >> specify
>> >> kernel_dir as above, this patch fixes this issue.
>> >> 
>> >> After this change, for normal build case, user can specify
>> >> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
>> >> as
>> >> kernel_dir. For cross compilation case, user can specify any
>> >> directory
>> >> that contains kernel source code as the kernel_dir.
>> >> 
>> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
>> >> Cc: 
>> >> stable@dpdk.org
>> >> 
>> >> Cc: 
>> >> iryzhov@nfware.com
>> >> 
>> >> 
>> >> Signed-off-by: Xiaolong Ye <
>> >> xiaolong.ye@intel.com
>> >
>> >The convention used by upstream and all distros is that kernel headers
>> >are in <version>/build. Why can't the cross compilation case also
>> >follow this convention, rather than adding complications to the
>> 
>> Yes, cross-compilation can follow this convention, but one common case is that
>> users download and put kernel src (the same kernel that's running in the target machine)
>> to one arbitrary dir, he then use this dir as kernel_dir to build kernel modules,
>> it's extra burden for users to create extra build dir to hold the kernel headers.
>> 
>
>As part of the build of the kernel, do you not do a "modules_install" step,
>which should set up things correctly for later builds?

Yes, this cmd helps. But for make build, user could specify both
/lib/modules/<kernelversion>/build and any kernel src dir as RTE_KERNELDIR, I
think this patch can help give user consistent experience when they migrate from make 
to meson build.

Thanks,
Xiaolong

>
>/Bruce

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

* Re: [dpdk-dev] [PATCH v3] kernel/linux: fix kernel dir for meson
  2019-12-08  1:26         ` Ye Xiaolong
@ 2019-12-09 12:12           ` Bruce Richardson
  0 siblings, 0 replies; 20+ messages in thread
From: Bruce Richardson @ 2019-12-09 12:12 UTC (permalink / raw)
  To: Ye Xiaolong; +Cc: Luca Boccassi, Ferruh Yigit, dev, stable, iryzhov

On Sun, Dec 08, 2019 at 09:26:57AM +0800, Ye Xiaolong wrote:
> On 12/04, Bruce Richardson wrote:
> >On Wed, Dec 04, 2019 at 10:18:21PM +0800, Ye Xiaolong wrote:
> >> On 12/04, Luca Boccassi wrote:
> >> >On Tue, 2019-12-03 at 23:59 +0800, Xiaolong Ye wrote:
> >> >> kernel_dir option in meson build is equivalent to RTE_KERNELDIR in
> >> >> make
> >> >> system, for cross-compilation case, users would specify it as local
> >> >> kernel src dir like
> >> >> 
> >> >> /<user local dir>/target-arm_glibc/linux-arm/linux-4.19.81/
> >> >> 
> >> >> Current meson build would fail to compile kernel module if user
> >> >> specify
> >> >> kernel_dir as above, this patch fixes this issue.
> >> >> 
> >> >> After this change, for normal build case, user can specify
> >> >> /lib/modules/<kernel_version> or /lib/modules/<kernel_version>/build
> >> >> as
> >> >> kernel_dir. For cross compilation case, user can specify any
> >> >> directory
> >> >> that contains kernel source code as the kernel_dir.
> >> >> 
> >> >> Fixes: 317832f97c16 ("kernel/linux: fix modules install path")
> >> >> Cc: 
> >> >> stable@dpdk.org
> >> >> 
> >> >> Cc: 
> >> >> iryzhov@nfware.com
> >> >> 
> >> >> 
> >> >> Signed-off-by: Xiaolong Ye <
> >> >> xiaolong.ye@intel.com
> >> >
> >> >The convention used by upstream and all distros is that kernel headers
> >> >are in <version>/build. Why can't the cross compilation case also
> >> >follow this convention, rather than adding complications to the
> >> 
> >> Yes, cross-compilation can follow this convention, but one common case is that
> >> users download and put kernel src (the same kernel that's running in the target machine)
> >> to one arbitrary dir, he then use this dir as kernel_dir to build kernel modules,
> >> it's extra burden for users to create extra build dir to hold the kernel headers.
> >> 
> >
> >As part of the build of the kernel, do you not do a "modules_install" step,
> >which should set up things correctly for later builds?
> 
> Yes, this cmd helps. But for make build, user could specify both
> /lib/modules/<kernelversion>/build and any kernel src dir as RTE_KERNELDIR, I
> think this patch can help give user consistent experience when they migrate from make 
> to meson build.
> 

Issues like specifying the wrong directory when switching over are
a) short term, once off issue in the conversion from make to meson and
b) fixed by just looking at the documentation for the build option.

On the other hand, maintaining code supporting both options is complexity
that, if added to meson builds, needs to be maintained going forward. While
we can expect users to have to make changes when switching build system, we
should not require them later to make changes based on us arbitrarily
changing the allowed values of options.

Therefore, unless your build system absolutely cannot be set up to support
the existing build configuration, I would rather we not accept changes
here, and we try and keep the code simpler.

Regards,
/Bruce

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

end of thread, other threads:[~2019-12-09 12:12 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02  6:14 [dpdk-dev] [PATCH] kernel/linux: fix kernel dir for meson Xiaolong Ye
2019-12-02  8:10 ` Igor Ryzhov
2019-12-02  8:39   ` Ye Xiaolong
2019-12-02  9:16     ` Igor Ryzhov
2019-12-02 11:34       ` Ye Xiaolong
2019-12-02 12:08         ` Bruce Richardson
2019-12-02 15:44           ` Ye Xiaolong
2019-12-03  5:33           ` Ye Xiaolong
2019-12-03 10:10             ` Bruce Richardson
2019-12-03  5:29 ` [dpdk-dev] [PATCH v2] " Xiaolong Ye
2019-12-03 10:11   ` Bruce Richardson
2019-12-03 12:33     ` Ye Xiaolong
2019-12-03 13:58       ` Bruce Richardson
2019-12-03 15:01         ` Ye Xiaolong
2019-12-03 15:59 ` [dpdk-dev] [PATCH v3] " Xiaolong Ye
2019-12-04 13:51   ` Luca Boccassi
2019-12-04 14:18     ` Ye Xiaolong
2019-12-04 15:12       ` Bruce Richardson
2019-12-08  1:26         ` Ye Xiaolong
2019-12-09 12:12           ` Bruce Richardson

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