Re: [dpdk-dev] [RFC PATCH v4] build: kni cross-compilation support

-----Original Message-----
From: Bruce Richardson <redacted>
Sent: Friday, February 5, 2021 4:27 PM
To: Juraj Linkeš <redacted>
Cc: thomas@monjalon.net; Ruifeng.Wang@arm.com;
Honnappa.Nagarahalli@arm.com; jerinjacobk@gmail.com;
hemant.agrawal@nxp.com; ferruh.yigit@intel.com; aboyer@pensando.io;
dev@dpdk.org; david.marchand@redhat.com; bluca@debian.org
Subject: Re: [RFC PATCH v4] build: kni cross-compilation support

On Fri, Feb 05, 2021 at 04:04:32PM +0100, Juraj Linkeš wrote:

quoted

The kni linux module is using a custom target for building, which
doesn't take into account any cross compilation arguments. The
arguments in question are ARCH, CROSS_COMPILE (for gcc, clang) and CC,
LD (for clang). Get those from the cross file and pass them to the
custom target.

The user supplied path may not contain the 'build' directory, such as
when using cross-compiled headers, so only append that in the default
case (when no path is supplied in native builds) and use the
unmodified path from the user otherwise. Also modify the install path

accordingly.

quoted

Signed-off-by: Juraj Linkeš <redacted>
---

Thanks, this all looks ok to me now, bar one very minor nit below. Doing a native
build on my system with the running kernel also works fine.

However, the bigger question is one of compatibility for this change. The current
documentation for the kernel_dir option is:
  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.')

Obviously the description now needs an update to reflect the new use

I'll change the description. The current patch version is always installing the modules into '/lib/modules/' + kernel_version + '/extra/dpdk', though. I don't think we want to change the behavior this way, so I'll make the changes to preserve to original behavior ('/lib/modules/' + kernel_version + '/extra/dpdk' when kernel_dir is not supplied, kernel_dir + '/extra/dpdk' when it is).

, but I'm
not sure if changing the behaviour counts as an "ABI" change or not, and
whether it needs to wait for a new LTS release. Any scripts that were compiling
using e.g. kernel_dir='/lib/modules/<version>' need to be changed to use
kernel_dir='/lib/modules/<version>/build' instead.

I'm not sure what to do with this. Should I make it backwards compatible by checking the build dir as well (i.e. trying make kernelversion in both $kernel_dir and $kernel_dir/build)?

/Bruce

quoted

 kernel/linux/kni/meson.build |  8 ++--
 kernel/linux/meson.build     | 80 ++++++++++++++++++++++++++++++------
 2 files changed, 71 insertions(+), 17 deletions(-)

diff --git a/kernel/linux/kni/meson.build

b/kernel/linux/kni/meson.build index 07e0c9dae7..46b71c7418 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_build_dir,
 		'M=' + meson.current_build_dir(),
 		'src=' + meson.current_source_dir(),
 		'MODULE_CFLAGS=-include ' + meson.source_root() +

'/config/rte_config.h' + @@ -21,8 +21,8 @@ custom_target('rte_kni',
 		' -I' + meson.source_root() + '/lib/librte_kni' +
 		' -I' + meson.build_root() +
 		' -I' + meson.current_source_dir(),
-		'modules'],
+		'modules'] + cross_args,
 	depends: kni_mkfile,
-	install: true,
-	install_dir: kernel_dir + '/extra/dpdk',
+	install: install,
+	install_dir: kernel_install_dir,
 	build_by_default: get_option('enable_kmods')) diff --git
a/kernel/linux/meson.build b/kernel/linux/meson.build index
5c864a4653..7acb52944f 100644

--- a/kernel/linux/meson.build
+++ b/kernel/linux/meson.build

@@ -3,25 +3,79 @@

 subdirs = ['kni']

-# if we are cross-compiling we need kernel_dir specified -if

get_option('kernel_dir') == '' and meson.is_cross_build()
-	error('Need "kernel_dir" option for kmod compilation when cross-

compiling')

quoted

-endif
+kernel_build_dir = get_option('kernel_dir') install = not
+meson.is_cross_build() cross_args = [] kernel_install_dir = ''

Minor nit, I'd have kernel_install_dir immediately after kernel_build_dir in the list
above.

Sure.