[dpdk-dev] [PATCH] kernel/linux: fix modules install path

[dpdk-dev] [PATCH] kernel/linux: fix modules install path

[Igor Ryzhov]

Currently kernel modules are installed into /usr/src/ instead of
/lib/modules when meson build system is used. This patch fixes that.

Old build option "kernel_dir" is changed to "kernel_version".

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
---
 kernel/linux/igb_uio/meson.build |  2 +-
 kernel/linux/kni/meson.build     |  2 +-
 kernel/linux/meson.build         | 16 +++++++++-------
 meson_options.txt                |  4 ++--
 4 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/kernel/linux/igb_uio/meson.build b/kernel/linux/igb_uio/meson.build
index f5a9d5ccf..5093610e3 100644
--- a/kernel/linux/igb_uio/meson.build
+++ b/kernel/linux/igb_uio/meson.build
@@ -16,5 +16,5 @@ custom_target('igb_uio',
 		'modules'],
 	depends: mkfile,
 	install: true,
-	install_dir: kernel_dir + '/../extra/dpdk',
+	install_dir: kernel_install_dir + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
index a9f48b0e6..8a902d2ed 100644
--- a/kernel/linux/kni/meson.build
+++ b/kernel/linux/kni/meson.build
@@ -25,5 +25,5 @@ custom_target('rte_kni',
 	depends: kni_mkfile,
 	console: true,
 	install: true,
-	install_dir: kernel_dir + '/../extra/dpdk',
+	install_dir: kernel_install_dir + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
index a37c95752..5a9303b33 100644
--- a/kernel/linux/meson.build
+++ b/kernel/linux/meson.build
@@ -3,19 +3,21 @@
 
 subdirs = ['igb_uio', 'kni']
 
-# if we are cross-compiling we need kernel_dir specified
-if get_option('kernel_dir') == '' and meson.is_cross_build()
-	warning('Need "kernel_dir" option for kmod compilation when cross-compiling')
+# if we are cross-compiling we need kernel_version specified
+if get_option('kernel_version') == '' and meson.is_cross_build()
+	warning('Need "kernel_version" option for kmod compilation when cross-compiling')
 	subdir_done()
 endif
 
-kernel_dir = get_option('kernel_dir')
-if kernel_dir == ''
-	# use default path for native builds
+kernel_version = get_option('kernel_version')
+if kernel_version == ''
+	# use default version for native builds
 	kernel_version = run_command('uname', '-r').stdout().strip()
-	kernel_dir = '/lib/modules/' + kernel_version + '/build'
 endif
 
+kernel_dir = '/lib/modules/' + kernel_version + '/build'
+kernel_install_dir = '/lib/modules/' + kernel_version
+
 # test running make in kernel directory, using "make kernelversion"
 make_returncode = run_command('make', '-sC', kernel_dir,
 		'kernelversion').returncode()
diff --git a/meson_options.txt b/meson_options.txt
index 16d9f92c6..5ca50d8dc 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -14,8 +14,8 @@ option('ibverbs_link', type: 'combo', choices : ['shared', 'dlopen'], value: 'sh
 	description: 'Linkage method (shared/dlopen) for Mellanox PMDs with ibverbs dependencies.')
 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, they will be installed in $DEST_DIR/$kernel_dir/../extra/dpdk')
+option('kernel_version', type: 'string', value: '',
+	description: 'kernel version for building kernel modules')
 option('lib_musdk_dir', type: 'string', value: '',
 	description: 'path to the MUSDK library installation directory')
 option('machine', type: 'string', value: 'native',
-- 
2.21.0

Re: [dpdk-dev] [PATCH] kernel/linux: fix modules install path

[Bruce Richardson]

On Mon, Jun 10, 2019 at 11:25:52AM +0300, Igor Ryzhov wrote:
> Currently kernel modules are installed into /usr/src/ instead of
> /lib/modules when meson build system is used. This patch fixes that.
> 
> Old build option "kernel_dir" is changed to "kernel_version".
> 
> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
> ---
>  kernel/linux/igb_uio/meson.build |  2 +-
>  kernel/linux/kni/meson.build     |  2 +-
>  kernel/linux/meson.build         | 16 +++++++++-------
>  meson_options.txt                |  4 ++--
>  4 files changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/linux/igb_uio/meson.build b/kernel/linux/igb_uio/meson.build
> index f5a9d5ccf..5093610e3 100644
> --- a/kernel/linux/igb_uio/meson.build
> +++ b/kernel/linux/igb_uio/meson.build
> @@ -16,5 +16,5 @@ custom_target('igb_uio',
>  		'modules'],
>  	depends: mkfile,
>  	install: true,
> -	install_dir: kernel_dir + '/../extra/dpdk',
> +	install_dir: kernel_install_dir + '/extra/dpdk',
>  	build_by_default: get_option('enable_kmods'))
> diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
> index a9f48b0e6..8a902d2ed 100644
> --- a/kernel/linux/kni/meson.build
> +++ b/kernel/linux/kni/meson.build
> @@ -25,5 +25,5 @@ custom_target('rte_kni',
>  	depends: kni_mkfile,
>  	console: true,
>  	install: true,
> -	install_dir: kernel_dir + '/../extra/dpdk',
> +	install_dir: kernel_install_dir + '/extra/dpdk',
>  	build_by_default: get_option('enable_kmods'))
> diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
> index a37c95752..5a9303b33 100644
> --- a/kernel/linux/meson.build
> +++ b/kernel/linux/meson.build
> @@ -3,19 +3,21 @@
>  
>  subdirs = ['igb_uio', 'kni']
>  
> -# if we are cross-compiling we need kernel_dir specified
> -if get_option('kernel_dir') == '' and meson.is_cross_build()
> -	warning('Need "kernel_dir" option for kmod compilation when cross-compiling')
> +# if we are cross-compiling we need kernel_version specified
> +if get_option('kernel_version') == '' and meson.is_cross_build()
> +	warning('Need "kernel_version" option for kmod compilation when cross-compiling')
>  	subdir_done()
>  endif

Looking at the patch now, I'm not sure that this change from kernel_dir to
kernel_version is the right thing to do - since it almost certainly cause
issues for cross-compiling. The kernel modules almost certainly won't be in
the host's /lib/modules folder in cross-compile cases, so I think we need
to continue to specify a path to the kernel modules folder.

To me the simplest option is that we should take the path to the kernels
module folder i.e. same as now, just without the "/build". A comment update
in the meson_options.txt file should be all that is needed to modify that
parameter, and we should be able to have the meson.build file automatically
strip "/build" off the path in order to enable backward compatibility. What
do you think?

/Bruce

Re: [dpdk-dev] [PATCH] kernel/linux: fix modules install path

[Igor Ryzhov]

Bruce,

From my understanding, kernel_dir is a directory with kernel headers needed
for modules building. When it's formed automatically, yes, it will be
"/lib/modules/version/build" and we can get installation directory by
stripping
"/build". But when it's set manually, it can be set to, for example,
"/usr/src/linux-headers-version", and build will be successful, but we
won't be
able to strip "/build" as there is no "/build".

Which path should be used for installation in cross-compile case, when the
kernel_dir is set manually?



On Mon, Jun 10, 2019 at 12:37 PM Bruce Richardson <
bruce.richardson@intel.com> wrote:

> On Mon, Jun 10, 2019 at 11:25:52AM +0300, Igor Ryzhov wrote:
> > Currently kernel modules are installed into /usr/src/ instead of
> > /lib/modules when meson build system is used. This patch fixes that.
> >
> > Old build option "kernel_dir" is changed to "kernel_version".
> >
> > Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
> > ---
> >  kernel/linux/igb_uio/meson.build |  2 +-
> >  kernel/linux/kni/meson.build     |  2 +-
> >  kernel/linux/meson.build         | 16 +++++++++-------
> >  meson_options.txt                |  4 ++--
> >  4 files changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/kernel/linux/igb_uio/meson.build
> b/kernel/linux/igb_uio/meson.build
> > index f5a9d5ccf..5093610e3 100644
> > --- a/kernel/linux/igb_uio/meson.build
> > +++ b/kernel/linux/igb_uio/meson.build
> > @@ -16,5 +16,5 @@ custom_target('igb_uio',
> >               'modules'],
> >       depends: mkfile,
> >       install: true,
> > -     install_dir: kernel_dir + '/../extra/dpdk',
> > +     install_dir: kernel_install_dir + '/extra/dpdk',
> >       build_by_default: get_option('enable_kmods'))
> > diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
> > index a9f48b0e6..8a902d2ed 100644
> > --- a/kernel/linux/kni/meson.build
> > +++ b/kernel/linux/kni/meson.build
> > @@ -25,5 +25,5 @@ custom_target('rte_kni',
> >       depends: kni_mkfile,
> >       console: true,
> >       install: true,
> > -     install_dir: kernel_dir + '/../extra/dpdk',
> > +     install_dir: kernel_install_dir + '/extra/dpdk',
> >       build_by_default: get_option('enable_kmods'))
> > diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
> > index a37c95752..5a9303b33 100644
> > --- a/kernel/linux/meson.build
> > +++ b/kernel/linux/meson.build
> > @@ -3,19 +3,21 @@
> >
> >  subdirs = ['igb_uio', 'kni']
> >
> > -# if we are cross-compiling we need kernel_dir specified
> > -if get_option('kernel_dir') == '' and meson.is_cross_build()
> > -     warning('Need "kernel_dir" option for kmod compilation when
> cross-compiling')
> > +# if we are cross-compiling we need kernel_version specified
> > +if get_option('kernel_version') == '' and meson.is_cross_build()
> > +     warning('Need "kernel_version" option for kmod compilation when
> cross-compiling')
> >       subdir_done()
> >  endif
>
> Looking at the patch now, I'm not sure that this change from kernel_dir to
> kernel_version is the right thing to do - since it almost certainly cause
> issues for cross-compiling. The kernel modules almost certainly won't be in
> the host's /lib/modules folder in cross-compile cases, so I think we need
> to continue to specify a path to the kernel modules folder.
>
> To me the simplest option is that we should take the path to the kernels
> module folder i.e. same as now, just without the "/build". A comment update
> in the meson_options.txt file should be all that is needed to modify that
> parameter, and we should be able to have the meson.build file automatically
> strip "/build" off the path in order to enable backward compatibility. What
> do you think?
>
> /Bruce
>
>

Re: [dpdk-dev] [PATCH] kernel/linux: fix modules install path

[Bruce Richardson]

On Mon, Jun 10, 2019 at 02:04:26PM +0300, Igor Ryzhov wrote:
>    Bruce,
>    From my understanding, kernel_dir is a directory with kernel headers
>    needed
>    for modules building. 

Right now, yes. I'd suggest that we change that to the actual kernel
modules directory, and we get both the build directory and the install
directory based off that.

>    When it's formed automatically, yes, it will be
>    "/lib/modules/version/build" and we can get installation directory by
>    stripping
>    "/build".

Well, I'd suggest if we query the value automatically we don't both adding
the build, and just add that later when building the modules, i.e.
kernel_dir should always be the the base directory without "build" on it.

>  But when it's set manually, it can be set to, for example,
>    "/usr/src/linux-headers-version", and build will be successful, but we
>    won't be
>    able to strip "/build" as there is no "/build".
>    Which path should be used for installation in cross-compile case, when
>    the
>    kernel_dir is set manually?

The stripping "build" was just a suggestion to allow the value to be
specified either with or without the "build/" suffix and have things work.
For the paths specified in the cross-compile case, my thinking was that we
would: 
* build using <kernel_dir>/build
* install to <kernel_dir>/extra/dpdk

as with the non-cross-compile case.

/Bruce

Re: [dpdk-dev] [PATCH] kernel/linux: fix modules install path

[Igor Ryzhov]

Thanks for the clarification, I get it now.
I will send v2 later today.

On Mon, Jun 10, 2019 at 2:12 PM Bruce Richardson <bruce.richardson@intel.com>
wrote:

> On Mon, Jun 10, 2019 at 02:04:26PM +0300, Igor Ryzhov wrote:
> >    Bruce,
> >    From my understanding, kernel_dir is a directory with kernel headers
> >    needed
> >    for modules building.
>
> Right now, yes. I'd suggest that we change that to the actual kernel
> modules directory, and we get both the build directory and the install
> directory based off that.
>
> >    When it's formed automatically, yes, it will be
> >    "/lib/modules/version/build" and we can get installation directory by
> >    stripping
> >    "/build".
>
> Well, I'd suggest if we query the value automatically we don't both adding
> the build, and just add that later when building the modules, i.e.
> kernel_dir should always be the the base directory without "build" on it.
>
> >  But when it's set manually, it can be set to, for example,
> >    "/usr/src/linux-headers-version", and build will be successful, but we
> >    won't be
> >    able to strip "/build" as there is no "/build".
> >    Which path should be used for installation in cross-compile case, when
> >    the
> >    kernel_dir is set manually?
>
> The stripping "build" was just a suggestion to allow the value to be
> specified either with or without the "build/" suffix and have things work.
> For the paths specified in the cross-compile case, my thinking was that we
> would:
> * build using <kernel_dir>/build
> * install to <kernel_dir>/extra/dpdk
>
> as with the non-cross-compile case.
>
> /Bruce
>
>

[dpdk-dev] [PATCH v2] kernel/linux: fix modules install path

[Igor Ryzhov]

Currently kernel modules are installed into /usr/src instead of
/lib/modules when meson build system is used. This patch fixes that.

Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
---
v2 - don't change kernel_dir to kernel_version

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

diff --git a/kernel/linux/igb_uio/meson.build b/kernel/linux/igb_uio/meson.build
index f5a9d5ccf..fac404f07 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,
+	command: ['make', '-C', kernel_dir + '/build',
 		'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: kernel_dir + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/kni/meson.build b/kernel/linux/kni/meson.build
index a9f48b0e6..955eec949 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,
+	command: ['make', '-j4', '-C', kernel_dir + '/build',
 		'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: kernel_dir + '/extra/dpdk',
 	build_by_default: get_option('enable_kmods'))
diff --git a/kernel/linux/meson.build b/kernel/linux/meson.build
index a37c95752..1796cc686 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 + '/build'
+	kernel_dir = '/lib/modules/' + kernel_version
 endif
 
 # test running make in kernel directory, using "make kernelversion"
-make_returncode = run_command('make', '-sC', kernel_dir,
+make_returncode = run_command('make', '-sC', kernel_dir + '/build',
 		'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 16d9f92c6..5302b9c68 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -15,7 +15,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, they will be installed in $DEST_DIR/$kernel_dir/../extra/dpdk')
+	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.')
 option('lib_musdk_dir', type: 'string', value: '',
 	description: 'path to the MUSDK library installation directory')
 option('machine', type: 'string', value: 'native',
-- 
2.21.0

Re: [dpdk-dev] [PATCH v2] kernel/linux: fix modules install path

[Bruce Richardson]

On Tue, Jun 11, 2019 at 11:49:34AM +0300, Igor Ryzhov wrote:
> Currently kernel modules are installed into /usr/src instead of
> /lib/modules when meson build system is used. This patch fixes that.
> 
> Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
> ---
> v2 - don't change kernel_dir to kernel_version
> 
>  kernel/linux/igb_uio/meson.build | 4 ++--
>  kernel/linux/kni/meson.build     | 4 ++--
>  kernel/linux/meson.build         | 4 ++--
>  meson_options.txt                | 2 +-
>  4 files changed, 7 insertions(+), 7 deletions(-)
> 
Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Re: [dpdk-dev] [PATCH v2] kernel/linux: fix modules install path

[Thomas Monjalon]

11/06/2019 12:11, Bruce Richardson:
> On Tue, Jun 11, 2019 at 11:49:34AM +0300, Igor Ryzhov wrote:
> > Currently kernel modules are installed into /usr/src instead of
> > /lib/modules when meson build system is used. This patch fixes that.
> > 
> > Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
> > ---
> > v2 - don't change kernel_dir to kernel_version
> > 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Applied, thanks